Skip to content

Improve isapprox #216

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 24, 2020
Merged

Improve isapprox #216

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 20 additions & 5 deletions src/FixedPointNumbers.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,31 @@ end
"""
isapprox(x::FixedPoint, y::FixedPoint; rtol=0, atol=max(eps(x), eps(y)))

For FixedPoint numbers, the default criterion is that `x` and `y` differ by no more than `eps`, the separation between adjacent fixed-point numbers.
For FixedPoint numbers, the default criterion is that `x` and `y` differ by no
more than `eps`, the separation between adjacent fixed-point numbers.
"""
function isapprox(x::T, y::T; rtol=0, atol=max(eps(x), eps(y))) where {T <: FixedPoint}
maxdiff = T(atol+rtol*max(abs(x), abs(y)))
rx, ry, rd = reinterpret(x), reinterpret(y), reinterpret(maxdiff)
abs(signed(widen1(rx))-signed(widen1(ry))) <= rd
@inline function isapprox(x::X, y::X; rtol=0, atol=eps(X)) where {X <: FixedPoint}
n, m = minmax(x, y) # m >= n
if rtol == zero(rtol)
_isapprox_atol(m, n, atol)
elseif atol == zero(atol)
_isapprox_rtol(m, n, rtol)
else
_isapprox_atol(m, n, atol) | _isapprox_rtol(m, n, rtol)
end
end
function isapprox(x::FixedPoint, y::FixedPoint; rtol=0, atol=max(eps(x), eps(y)))
isapprox(promote(x, y)...; rtol=rtol, atol=atol)
end
function _isapprox_atol(m::X, n::X, atol::X) where {X <: FixedPoint}
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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@kimikage kimikage Aug 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

end
function _isapprox_rtol(m::X, n::X, rtol) where {X <: FixedPoint}
unsigned(m.i - n.i) <= rtol * max(unsigned(abs(m.i)), unsigned(abs(n.i)))
end

# predicates
isinteger(x::FixedPoint) = x == trunc(x) # TODO: use floor(x) when dropping support for Fixed{Int8,8}
Expand Down
9 changes: 9 additions & 0 deletions test/fixed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,15 @@ end
@test all(x -> x + eps(F) ≈ x, xs)
@test !any(x -> x - eps(F) ≈ x + eps(F), xs)
end

@test isapprox(-0.5Q0f7, -1Q0f7, rtol=0.5, atol=0) # issue 209
@test isapprox(typemin(Q0f7), typemax(Q0f7), rtol=2.0)
@test !isapprox(zero(Q0f7), typemax(Q0f7), rtol=0.9)
@test isapprox(zero(Q0f7), eps(Q0f7), rtol=1e-6) # atol = eps(Q0f7)
@test !isapprox(eps(Q0f7), zero(Q0f7), rtol=1e-6, atol=1e-6)
@test !isapprox(1.0Q6f1, 1.5Q6f1, rtol=0.3, atol=0) # 1.5 * 0.3 < eps(Q6f1)

@test isapprox(eps(Q8f7), eps(Q0f7), rtol=1e-6)
end

@testset "clamp" begin
Expand Down
8 changes: 8 additions & 0 deletions test/normed.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,14 @@ end
@test all(x -> x + eps(N) ≈ x, xs)
@test !any(x -> x - eps(N) ≈ x + eps(N), xs)
end

@test isapprox(typemin(N0f8), typemax(N0f8), rtol=1.0)
@test !isapprox(zero(N0f8), typemax(N0f8), rtol=0.9)
@test isapprox(zero(N0f8), eps(N0f8), rtol=1e-6) # atol = eps(N0f8)
@test !isapprox(eps(N0f8), zero(N0f8), rtol=1e-6, atol=1e-6)
@test !isapprox(0.66N6f2, 1.0N6f2, rtol=0.3, atol=0) # 1.0 * 0.3 < eps(N6f2)

@test isapprox(eps(N8f8), eps(N0f8), rtol=1e-6)
end

@testset "comparison" begin
Expand Down