Fix minimum/maximum over dimensions with missing values

[nalimilan]

v0 != v0 returns missing for missing values. Use the largest/smallest non-missing value to initialize the array. This is an inefficient approach. Faster alternatives would be to avoid using an initial value at all, and instead keep track of whether a value has been set in a separate mask; or to use typemax/typemin for types that support them.

Fixes #35308.

[JeffBezanson]

I think we should aim for a better fix --- ideally, no element of the array should come into contact with an unnecessary value. E.g. when reducing along row 1 no values from other rows should be relevant.

[nalimilan]

Well the result follows that rule. It's just that the implementation is simpler if you take the largest value of the array (resp. the smallest) to init all slices when computing the minimum (resp. maximum), but it's not visible to the user.

Actually I have a better implementation which looks at the first non-missing value of each slice, but it takes about 50 lines (it's very similar to nonmissingval at https://github.com/JuliaLang/julia/pull/28027/files#diff-c4a56019a8dae547ca5556ad8df4d1f0R384). Which approach do you prefer?

[tkf]

If you use an external boolean array to keep the "stage" of reduction (that's my understanding of how nonmissingval works), using _InitialValue + BottomRF as now done in foldl might be simpler. However, this would need to use Array{Union{T,Missing,_InitialValue}} as the destination/state. It would be nice if there is an API to get Array{Union{T,Missing}} from Array{Union{T,Missing,_InitialValue}} by only re-writing the type tag part of the array. Is there such an API? (Edit: I just realized this is more about a comment on nonmissingval. Re-posted here: #28027 (comment))

[nalimilan]

@JeffBezanson What's your preferred approach given my last comment?

[nalimilan]

Bump.

[stev47]

seems to be fixed by 76952a8

[nalimilan]

Not completely unfortunately. See the note there:

                # TODO: Some types, like BigInt, don't support typemin/typemax.
                # So a Matrix{Union{BigInt, Missing}} can still error here.

[LilithHafner]

I agree with @JeffBezanson that we should have a better fix. I think we should not use typemin/typemax/initial values at all in the non-empty case.

However, this is a bugfix so we should merge it to have fewer bugs until someone contributes a better fix.

The tests this add already pass on master, so we should also add tests with BigFloat or BigInt that currently fail.

Also needs a rebase.

[nalimilan]

I've rebased locally, but I'm not sure what's the best thing to do: should I keep the inefficient approach used here for all cases (this is slow only for one slice so not so slow for large arrays), or be smarter and try to use typemin/typemax for types that support it like #35316 and #43604? AFAICT the latter would imply using try... catch and falling back to the former in case of failure (using applicable would also be an option). What do you think?

[LilithHafner]

IMO it is best to use typemin/typemax only on known types (e.g. Union{Base.BitInteger, Base.IEEEFloat}). Long term we should be able to get just as fast or faster without them at all, so I don't think this is worth a trait. applicable is unreliable (though I suspect it would be pretty accurate in this case) and I doubt try/catch would be good for performance.

[nalimilan]

OK, I've opted for Union{BitInteger, IEEEFloat, BigFloat}, as BigFloat supports Inf. I was even tempted to use AbstractFloat but it's not completely clear whether that interface requires supporting Inf or not.

[nalimilan]

@LilithHafner Good to go now?

[LilithHafner]

I don't love the fallback initialization that ends up iterating over the first slice up to 3 times; or this whole system (that existed before this PR). It seems more complex than it needs to be and less performant than it could be. However, bugfix=merge.

[nalimilan]

Yeah clearly this approach isn't great... But redesigning all this code isn't trivial. :-/