Skip to content
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.

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

isapprox is incorrect for tolerances beyond standard precision #206

Open
tobydriscoll opened this issue Aug 5, 2024 · 4 comments
Open

Comments

@tobydriscoll
Copy link

The following is incorrect:

julia> T = Double64
Double64 (alias for DoubleFloat{Float64})

julia> x = Double64(0.9, 1e-20)
9.00000000000000022214460492503130852e-01

julia> abs(x - T(9)/10)
2.22144604925031320405193304841894479e-17

julia> isapprox(x, T(9)/10; rtol=1e-22, atol=1e-22)
true

If I understand the internals, the isapprox function uses HI(x)===HI(y) as a shortcut for the test, which doesn't look right.

@JeffreySarnoff
Copy link
Member

JeffreySarnoff commented Aug 5, 2024 via email

@tobydriscoll
Copy link
Author

The most straightforward thing would be to remove HI(x) === HI(y) || from line 20 of misc.jl. I suppose you could keep it by pairing it with a test that rtol is large enough, but I'm skeptical that the potential time savings is worth very much.

And then. of course, adding some relevant unit tests. 😁

@JeffreySarnoff
Copy link
Member

JeffreySarnoff commented Aug 6, 2024 via email

@tobydriscoll
Copy link
Author

OK, if the HI parts of x and y are the same, then x-y is equal to x.lo-y.lo, right? So that's cheap.

But I don't know if you can cheat on the rtol*max(abs(x), abs(y)) part if rtol is specified in the same precision as x and y. The chances that you would care about the difference between using, say, abs(x) and abs(x.hi) there seem remote, but you can probably invent a pathological counterexample that would break the implied guarantee.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants