fix type of constants in ported sincosf #331
Conversation
src/math/sincosf.rs
Outdated
| // use crate::_eqf; | ||
|
|
||
| // #[test] | ||
| // fn with_pi() { |
There was a problem hiding this comment.
Does pi not pass anymore?
There was a problem hiding this comment.
Because the patched function now produces more accurate results, (sincosf(core::f32::consts::PI) = (-8.742278e-8, -1) ), the old test doesn't pass. I'm not really sure what to do with it.
There was a problem hiding this comment.
Oh, interesting. Maybe just delete the test and leave a NB comment for the module, in case we replace this with an implementation that handles it (or just special cases?)
|
Thanks, does this lower the ULP error around multiples of pi/2? |
|
Yes. I've tested it on your |
|
Oh, awesome! Thanks for being thorough. |
|
Thanks! |
When porting sincosf from musl, the double-precision constants used for range reduction were mistakenly (I assume) cast to f32, which led to inaccurate results in some cases.