Array isapprox behavior when the norm is NaN

[ararslan]

When using isapprox to compare arrays, the current definition, introduced in #12472, seems to have some (IMHO) unideal behavior in the presence of NaNs:

julia> [0.01311489462160816,Inf] ≈ [0.013114894621608135,Inf]
false

julia> all(map(isapprox, [0.01311489462160816,Inf], [0.013114894621608135,Inf]))
true

This is how isapprox is currently defined for arrays:

function isapprox{T<:Number,S<:Number}(x::AbstractArray{T}, y::AbstractArray{S}; rtol::Real=Base.rtoldefault(T,S), atol::Real=0, norm::Function=vecnorm)
    d = norm(x - y)
    return isfinite(d) ? d <= atol + rtol*max(norm(x), norm(y)) : x == y
end

Whenever norm evaluates to NaN, which happens for example when x and y both have Inf in a corresponding index, the comparison falls back to checking exact equality using ==. That will be false if there are finite, approximately equal floating point values in other corresponding indices in x and y. This is causing a test failure in HypothesisTests.jl.

@stevengj was against checking approximate equality element-wise for arrays but I'm not entirely clear on why. Is there anything that can or should be changed in the current implementation, or is there some other way I can get around the test failure?

[KristofferC]

If you look at an array as a collection of random stuff then element-wise seems sensible. If you look at an array as an element of a vector space then norm seems sensible.

Maybe there should be .≈ as a way of writing element-wise approx?

[tkelman]

will we get that automatically from #17623?

[ararslan]

@KristofferC Definitely a good point. I don't see why checking for exact equality in an approximate comparison when the vector norm is NaN would be the best course of action though. Maybe we could fall back to an element-wise comparison in that case rather than checking ==? ¯_(ツ)_/¯

[KristofferC]

That seems reasonable to me.

[ararslan]

Okay, cool! Thanks so much for the feedback, @KristofferC, I appreciate it. If there's support for that approach then I can put together a PR. I'd be interested to hear from @stevengj and @timholy too.

[stevengj]

The question in any approximate comparison is "small difference compared to what?" For almost any function that produces a vector, elementwise isapprox is too strong a condition, and some kind of vector norm is more appropriate. For example, imagine comparing ifft(fft(x)) to x when one of the elements of x is zero.

It's not completely clear what you want to do if one of the elements is Inf; maybe there is no good default here.

If you want elementwise isapprox, very soon (in 0.6) you'll be able to use all(...) with the dotted isapprox operator.

[tkelman]

And it should already work now as all(isapprox.(a, b)), right?

[ararslan]

Yes

[ararslan]

For almost any function that produces a vector, elementwise isapprox is too strong a condition

Not as strong as == though, which is why it seems like it would be a better fallback in the presence of Inf and friends.

[stevengj]

Fair enough, as a fallback it wouldn't be terrible.

[ararslan]

Do you think it at least makes more sense as a fallback than ==?

[stevengj]

Yeah, sure.

[ararslan]

Okay, I'll put together a PR then. Thanks for your help, @stevengj, I appreciate it!