Skip to content

Merge m_atan2() and c_atan2() helper functions (or replace libm's atan2) #122681

Closed
@skirpichev

Description

@skirpichev

Feature or enhancement

Proposal:

With #29179, many workarounds for buggy libm implementations were removed. Yet, there is m_atan2():

static double
m_atan2(double y, double x)
{
if (isnan(x) || isnan(y))
return Py_NAN;
if (isinf(y)) {
if (isinf(x)) {
if (copysign(1., x) == 1.)
/* atan2(+-inf, +inf) == +-pi/4 */
return copysign(0.25*Py_MATH_PI, y);
else
/* atan2(+-inf, -inf) == +-pi*3/4 */
return copysign(0.75*Py_MATH_PI, y);
}
/* atan2(+-inf, x) == +-pi/2 for finite x */
return copysign(0.5*Py_MATH_PI, y);
}
if (isinf(x) || y == 0.) {
if (copysign(1., x) == 1.)
/* atan2(+-y, +inf) = atan2(+-0, +x) = +-0. */
return copysign(0., y);
else
/* atan2(+-y, -inf) = atan2(+-0., -x) = +-pi. */
return copysign(Py_MATH_PI, y);
}
return atan2(y, x);
}

and
static double
c_atan2(Py_complex z)
{
if (isnan(z.real) || isnan(z.imag))
return Py_NAN;
if (isinf(z.imag)) {
if (isinf(z.real)) {
if (copysign(1., z.real) == 1.)
/* atan2(+-inf, +inf) == +-pi/4 */
return copysign(0.25*Py_MATH_PI, z.imag);
else
/* atan2(+-inf, -inf) == +-pi*3/4 */
return copysign(0.75*Py_MATH_PI, z.imag);
}
/* atan2(+-inf, x) == +-pi/2 for finite x */
return copysign(0.5*Py_MATH_PI, z.imag);
}
if (isinf(z.real) || z.imag == 0.) {
if (copysign(1., z.real) == 1.)
/* atan2(+-y, +inf) = atan2(+-0, +x) = +-0. */
return copysign(0., z.imag);
else
/* atan2(+-y, -inf) = atan2(+-0., -x) = +-pi. */
return copysign(Py_MATH_PI, z.imag);
}
return atan2(z.imag, z.real);
}

They are identical, the second one just uses Py_complex to keep arguments.

We should either 1) move first helper to _math.h and reuse it in the cmathmodule.c or 2) just remove these helpers. I'll provide patch for 1) proposal.

But 2) looks also safe for me: it seems that c_atan2() helper was used only for cmath.phase() and cmath.polar(); libm's atan2() used for the rest - and this don't poses any problems in CI or build bots. ("Problem" values are tested, e.g. infinities and nans for acos().)

Has this already been discussed elsewhere?

This is a minor feature, which does not need previous discussion elsewhere

Links to previous discussion of this feature:

No response

Linked PRs

Metadata

Metadata

Assignees

No one assigned

    Labels

    type-featureA feature request or enhancement

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions