Improve isapprox
#216
Improve isapprox
#216
Conversation
Codecov Report
@@ Coverage Diff @@
## master #216 +/- ##
==========================================
+ Coverage 88.55% 89.24% +0.68%
==========================================
Files 6 6
Lines 498 502 +4
==========================================
+ Hits 441 448 +7
+ Misses 57 54 -3
Continue to review full report at Codecov.
|
This changes the association between absolute tolerance and relative tolerance from additive (`+`) to `max` to match the behavior in `Base` of Julia v1. This also fixes an overflow problem and a rounding problem. This speeds up the operation with default tolerance settings.
Benchmarkjulia> versioninfo()
Julia Version 1.5.0
Commit 96786e22cc (2020-08-01 23:44 UTC)
Platform Info:
OS: Windows (x86_64-w64-mingw32)
CPU: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz
WORD_SIZE: 64
LIBM: libopenlibm
LLVM: libLLVM-9.0.1 (ORCJIT, skylake)
julia> x_n0f8 = collect(rand(N0f8, 1024, 1024)); y_n0f8 = collect(rand(N0f8, 1024, 1024));
julia> x_q0f7 = collect(rand(Q0f7, 1024, 1024)); y_q0f7 = collect(rand(Q0f7, 1024, 1024));
julia> @btime isapprox.($x_n0f8, $y_n0f8);
3.226 ms (4 allocations: 132.31 KiB) # before
140.900 μs (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_q0f7, $y_q0f7);
156.800 μs (4 allocations: 132.31 KiB) # before
140.801 μs (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_n0f8, $y_n0f8; atol=0.1);
3.583 ms (4 allocations: 132.31 KiB) # before
140.700 μs (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_q0f7, $y_q0f7; atol=0.1);
156.700 μs (4 allocations: 132.31 KiB) # before
140.901 μs (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_n0f8, $y_n0f8; atol=0, rtol=0.1);
3.289 ms (4 allocations: 132.31 KiB) # before (somewhat broken)
3.906 ms (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_q0f7, $y_q0f7; atol=0, rtol=0.1);
4.384 ms (4 allocations: 132.31 KiB) # before (somewhat broken)
4.132 ms (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_n0f8, $y_n0f8; rtol=0.1); # This behaves differently before and after this PR.
3.286 ms (4 allocations: 132.31 KiB) # before
560.699 μs (4 allocations: 132.31 KiB) # after
julia> @btime isapprox.($x_q0f7, $y_q0f7; rtol=0.1); # This behaves differently before and after this PR.
4.435 ms (4 allocations: 132.31 KiB) # before
561.299 μs (4 allocations: 132.31 KiB) # after
In this benchmark, the results are in |
| unsigned(m.i - n.i) <= unsigned(max(atol, zero(X)).i) | ||
| end | ||
| function _isapprox_atol(m::X, n::X, atol) where {X <: FixedPoint} | ||
| unsigned(m.i - n.i) <= div(atol, eps(X)) |
There was a problem hiding this comment.
I'm surprised this div isn't slow. I know that since the denominator is a compile-time constant there are some optimizations available, but I am definitely surprised.
There was a problem hiding this comment.
Because atol does not depend on the elements of the array, it can be calculated outside the loop. That's not surprising, but it's surprising that Julia understands that. 😄
BTW, division is not so slow in Skylake and later generations. (In the case of the div for floating point numbers, the function call of fmod messes it up, though. 😭) It is probably more efficient to use rawone than eps, but Q0f7, for example, cannot use rawone.
|
Thanks for your review. |
This changes the association between absolute tolerance and relative tolerance from additive (
+) tomaxto match the behavior inBaseof Julia v1 (cf. #209 (comment)).This also fixes an overflow problem and a rounding problem.
The use of
isapproxin a "strict" manner is rare, and I think the impact of these breaking changes are actually minor. However, these changes are difficult to notice and can affect the test results, so I labeled this PR with the "breaking" label as a marker.This also speeds up the operation with default tolerance settings.
Fixes #209