Re: [PATCH v2 guile-lib] src: (math trigonometry): Add it.

Adam Faiz <[email protected]> Fri, 21 Feb 2025 17:57:57 +0800
Newsgroups gmane.lisp.guile.devel
Message-ID <[email protected]>
> atan(0,0) is not well-defined (more precisely, there is no unique choice within the interval [0,360°)) - if the point (0,0) is written in polar coordinates (0*cos theta, 0*sin theta), then any angle theta can be used.
> 
> 
> If you intend to support (arctan2 0 0), it may be good to mention that in the docstring, and make sure it is in {0,180,-180} to match IEEE, and add a test for x=y=0.

Right, I've documented and tested that it returns 0 for (arctan2 0 0).

> 
> 
> I'm not sure, but I think the inexact +0.0 and -0.0 is counted as 'zero?', which might not be what you want it to do here. Being an inexact input (representing a number near 0 but not necessarily exactly 0), and since 'atan2' is not locally constant at 0, the output would need to be inexact too. Or, maybe you could let it return something exact anyways, idk. Exactness/inexactness considerations can become a bit arbitrary sometimes - sometimes '+0.0' is _exactly_ positive zero).

I'll stick with keeping the result exact, I don't think there's much else that can be inferred when the number is zero but inexact.

> 
> 
> If you consider signed infinity to simply be the exact signed infinity, you could also make (atan2 some-signed-infinity x) and (atan2 y some-signed-infinity) exact. (If it instead is considered inexact, as in maybe exact signed infinity, maybe a very large value, then this doesn't follow).

atan2 doesn't do anything special for now, I'm not 100% sure how much of an improvement it would be to specifically handle infinity values.

> 
> 
> In the new tests, it appears that you are only testing whether the values (approximately) match, and not whether return values are exact where they can be. Perhaps you could modify 'within-margin?' to do an exactness check when the reference value is exact?

Sure.

> 
> 
> For the tangent, tan(45°) is rational despite sin(45°) and cos(45°) being irrational. So, 'tangent' needs to be adjusted. Also, you should consider what to do with (tangent 45) - return +infinity or raise an exception for divide-by-zero? Either way, it's something to document.

I've documented that it raises an exception for divide-by-zero.

> 
> 
> For another test, you could check that (map (arcsine sine) list-of-angles) equals list-of-angles (with exact comparison whenever any of the compared things is exact, and approximate if both are inexact). Likewise for (map (sine arcsine) list-of-ratios). This can then easily be repeated for cosine and tangent (*).
> 
> 
> (*) If 45° is included in the angles: assuming (tangent 90) returns +infinity and (arctan +infinity) returns the exact 90. In case of divide-by-zero exception, or (arctan +infinity) being inexact 90.0, this wouldn't work as-is.
> 
> 
>     +  (< (sqrt (expt (- value estimate) 2)) margin))
> 
> 
> That's simply (< (abs (- value estimate)) margin), no? Or does Guile not have 'abs'?

Yes, I forgot that it existed. I've switched to using it in the revised patch.