-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add isapprox for arrays, with ≈ and ≉ synonyms #12472
Conversation
As an added benefit, now if you do And you can also chain |
CI is screwed up with build number collisions (appveyor/ci#353) and a mac freeze, can you rebase and force push this branch again? |
Thanks, @tkelman. I needed to update NEWS anyway. |
The thing that bothers me about this is that it seems very common to need to tweak the behavior of |
@StefanKarpinski, if you look at our own |
I also just grepped for all usages of |
Perhaps, but then why do we always seem to have random tests that fail because their tolerances are too tight? cc: @jiahao, @andreasnoack (who probably have the best sense of this) |
We (read: @stevengj) did just change the default tolerances to Better Values, so perhaps this is less of an issue now? |
I'd be thrilled be wrong about this, I just thought it worth bringing up as a concern. |
@StefanKarpinski's observation is closely related to JuliaLang/LinearAlgebra.jl#67 - many of the linear algebra tests (and probably tests for other numeric features too) use very loose tolerances that are not theoretically rigorous, and so in many cases are severe overestimates but in some other cases may be underestimates. |
Rebased. |
Since you guys are the ones who actually use |
@jiahao, should I go ahead and merge once the tests are green? |
Sure |
(My feeling is that most |
add isapprox for arrays, with ≈ and ≉ synonyms
@@ -2935,11 +2935,18 @@ cis | |||
doc""" | |||
```rst | |||
:: | |||
isapprox(x::Number, y::Number; rtol::Real=sqrt(eps), atol::Real=0) | |||
isapprox(x, y; rtol::Real=sqrt(eps), atol::Real=0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
signature may need to be updated to match this in the RST docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, fixed.
add AbstractFloat (fix #121)
add AbstractFloat (fix #121)
As discussed in #12393, this adds an
isapprox(x,y; rtol=sqrt(eps), atol=0, norm=vecnorm)
function for arrays, analogous toisapprox
for numbers. The definition is exactly the same except thatabs
is replaced bynorm
, and you can specify a norm via thenorm
keyword argument.Rationale: this is the most natural extension of
isapprox
to arrays and shares the same definition, is a useful function for arrays (especially for test programs), and people often get it wrong (e.g. FactCheck.jl arguably got it wrong in theirroughly
function as discussed in #12393, which checksisapprox
pointwise — this is almost always the wrong thing to do).This PR also adds synonyms
≈
(U+2248, LaTeX\approx
) and≉
(U+2249, LaTeX\napprox
) forisapprox
and!isapprox
, respectively. The official definitions of these symbols ("almost equal to") and the LaTeX abbreviations both mirror ourisapprox
function, so it is not really reasonable for people to define these to be any other function (and if you do need other notions of approximate equality there are other available Unicode operators, e.g. U+2245). And the binary operator is a lot more readable thanisapprox
(as long as you are satisfied with the default tolerances).