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

replace @test_approx_eq_eps with another idiom #19899

Closed
StefanKarpinski opened this issue Jan 6, 2017 · 9 comments
Closed

replace @test_approx_eq_eps with another idiom #19899

StefanKarpinski opened this issue Jan 6, 2017 · 9 comments
Labels
status:help wanted Indicates that a maintainer wants help on an issue or pull request testsystem The unit testing framework and Test stdlib

Comments

@StefanKarpinski
Copy link
Sponsor Member

Part of #4615. Now that @test_approx_eq has been deprecated and removed from our test suite, the next step is to replace all uses of @test_approx_eq_eps from the test suite and deprecate that macro in a similar fashion. Possible replacement for @test_approx_eq_eps a b ɛ:

@test (a, b, atol = ɛ)

I kind of wish there was a nicer way to write that, but I'm not sure that there is.

@StefanKarpinski StefanKarpinski added testsystem The unit testing framework and Test stdlib status:help wanted Indicates that a maintainer wants help on an issue or pull request labels Jan 6, 2017
@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 6, 2017

@test abs(a - b) < ɛ ?

@StefanKarpinski
Copy link
Sponsor Member Author

One idea: change the @test macro (and @test_{throws,broken,skip} too) to accept zero or more trailing arguments of the form x=y which, if the test if a call form, are passed as keyword arguments to the call, even when it's not expressed using call syntax. That would allow writing:

@test a  b atol=ɛ

@StefanKarpinski
Copy link
Sponsor Member Author

StefanKarpinski commented Jan 6, 2017

abs is no longer automatically vectorized and < never has been, so it would have to be more like:

@test maximum(abs, a - b) < ɛ

Perhaps that's better since it's far more explicit about what the test is actually checking.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 6, 2017

Or maybe reduce(maxabs, a - b)?

@andreasnoack
Copy link
Member

norm?

@StefanKarpinski
Copy link
Sponsor Member Author

We also deprecated maxabs, so that won't work either.

@StefanKarpinski
Copy link
Sponsor Member Author

StefanKarpinski commented Jan 6, 2017

norm might be the right thing to check, but that's certainly not what the tests currently do, so that would not be a straightforward search-and-replace by any means.

StefanKarpinski added a commit that referenced this issue Jan 6, 2017
StefanKarpinski added a commit that referenced this issue Jan 6, 2017
StefanKarpinski added a commit that referenced this issue Jan 6, 2017
StefanKarpinski added a commit that referenced this issue Jan 6, 2017
StefanKarpinski added a commit that referenced this issue Jan 6, 2017
Note that tolerance of the first Anscombe's quartet test in
test/linalg/generic.jl had to be loosened from 10e-5 to 15e-5
because ≈ uses the norm of the difference instead of the maximum
absolute difference, which is a stricter test.

Closes #19899.
@ararslan
Copy link
Member

ararslan commented Jan 6, 2017

Would @test isapprox(a, b, atol=ɛ) not suffice? It's just the tolerance-provided version of , which we're already using in place of @test_approx_eq.

@StefanKarpinski
Copy link
Sponsor Member Author

Yes, I was just proposing a slightly nicer syntax for that kind of thing. Not sure if it's worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:help wanted Indicates that a maintainer wants help on an issue or pull request testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

No branches or pull requests

4 participants