Reduce tols in sin/cos/tan tests - #1479
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1479 +/- ##
=========================================
- Coverage 98.7% 98.7% -0.0%
=========================================
Files 315 315
Lines 26457 26449 -8
Branches 2253 2253
=========================================
- Hits 26112 26104 -8
Misses 345 345
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
| @@ -75,7 +56,7 @@ | |||
| std::random_device rd; | |||
| std::mt19937_64 gen(rd()); | |||
There was a problem hiding this comment.
rd() is not used if we reset the seed after the constructor
There was a problem hiding this comment.
I'd like it if we used a fixed seed here in the testing so we don't get mystery failures or spurious coverage changes.
| // https://www.boost.org/LICENSE_1_0.txt | ||
|
|
||
| #include "testing_config.hpp" | ||
| #include <chrono> |
There was a problem hiding this comment.
Since time_point removed, no other usages in <chrono> now
I try (whenever refactoring a function or its tests) to use fixed seeds. I have grown to agree strongly with the fixed seed approach. Unfortunately, I missed the one mentioned here and it should be changed. Hi @ibmibmibm when we refactor functions and tests, repülace |
Yes please, just do it for us at your convenience. Cc: @mborland |
|
You can mix the cosine tolerance reduction with generally any and all refactoring of the trigonometric elementary functions sine, cosine, tangent, arcsine, arccosine and arctangent. Finish it. |
The random cos test now uses 16 float eps, the same as the sin test in boostorg#1479. Over 200000 random points the largest cos error is 5 eps. The asin edge test now seeds its generator with a fixed value instead of the clock, so a failure is reproducible.
The random cos test now uses 16 float eps, the same as the sin test in boostorg#1479. Over 200000 random points the largest cos error is 5 eps. The asin edge test now seeds its generator with a fixed value instead of the clock, so a failure is reproducible.
This PR follows the review of #1479. `test_cos()` in `test/test_sin_cos.cpp` now uses a tolerance of 16 float eps, the same value that #1479 uses for `test_sin()`. The old value was 35 eps. Over 200000 random points in [-2π, 2π], the largest cos error for all decimal types is 5 eps. `test_asin_edge()` in `test/test_asin.cpp` now seeds its generator with a fixed value. The old code seeded it from the clock, so a failure was not reproducible. This change also removes the unused `time_point()` helper, `std::random_device`, and `<chrono>`. This PR does not change `test/test_tan.cpp` because #1479 already changes the seed there. Both tests pass with GCC and Clang.
3 participants
This PR reduces the tolerances in
sin(),cos(),tan()testing based on improvements in #1475