[RFC]: add `math/base/special/acotf`
Closed this issue · 17 comments
Description
This RFC proposes adding math/base/special/acotf, which would be the single-precision equivalent of math/base/special/acot.
float stdlib_base_acotf( const float x )Related Issues
None.
Questions
No.
Other
No.
Checklist
- I have read and understood the Code of Conduct.
- Searched for existing issues and pull requests.
- The issue name begins with
RFC:.
I'd like to work on this.
Hey @kgryte, while implementing this, I got to know that, since we require atanf for this, and upon passing -infinity to acotf, or -0 to atanf, our atanf returns 0, instead of -0. This applies for both C and JavaScript implementations of atanf.
This leads to the failure of one particular test case, which checks if the function returns "-0" if provided "-infinity".
In the case of current acot, this problem doesn't arise, but it does come up in case of acotf.
Which behavior is the correct behavior?
The correct behavior is that of acot, i.e, it returns -0 when provided -infinity.
Applying the same implementation for acotf, this fails.
The question is more fundamental. Should atan distinguish between signed zeros? This is a mathematical question.
It's possible, e.g., that the tests for acot are incorrect, but this gets back to what the behavior of atan should be.
We get our answer from http://www.dsc.ufcg.edu.br/~cnum/modulos/Modulo2/IEEE754_2008.pdf
For the functions expm1, exp2m1, exp10m1, logp1, log2p1, log10p1, sin, tan, sinPi, atanPi, asin, atan, sinh, tanh, asinh, and atanh, f (+0) is +0 and f (−0) is −0 with no exception.
Meaning, atanf is incorrect here.
When I checked logging the values with atan, it returns:
atan( -0 ) = -0
atan( +0 ) = 0
Yes, but we should not just rely on what the current behavior is in stdlib, as the current behavior could be wrong. In this case, we need to consult the specification to ensure that our current atan behavior adheres to IEEE 754 semantics.
In this case, it does. Which just means that our current atanf implementation does not handle signed zero correctly.
Yes, and also, upon doing the same for atanf, it gives 0 in both cases.
We might need to modify our atanf implementation.
The reason it doesn't is pretty straightforward. We just need to make a small modification to atanf which we can observe in atan. Namely, the following
should be modified to
if ( isnanf( x ) ) {
+ if ( isnanf( x ) || x === 0.0 ) {
+ return x;This is a bug in the original Cephes implementation. You'll want to fix in both our JS and C atanf implementations.
When you make patch, can you also add corresponding tests to atanf?
Sure, I'll do it. Should this be added in the same PR as that of acotf, or a seperate one ?
I suggest making a quick patch in a separate PR to keep things clean.
Great, I'll follow that. Thanks for clearing this out.
This can be a nice example for why we shouldn't rely on just one source, like cephes.
Indeed! And the importance of tests! We are apparently missing corresponding tests in atan, as well. You can include those in your patch.