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

On 17/02/2025 8:03, Adam Faiz wrote:

> +  (cond ((zero? x)
> +         (let ((sign (if (positive? y) + -)))
> +           (sign 90)))

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.

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).

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).

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?

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.

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'?

Best regards,
Maxime Devos