gh-153144: Avoid checking `errno` for `atan2` by hpkfft · Pull Request #153148 · python/cpython

hpkfft · 2026-07-05T19:19:07Z

The C23 standard states that for atan2 and atan2pi:

A domain error may occur if both arguments are zero.
A range error occurs if x is positive and nonzero y/x is too close to zero.

Since Python should not raise ValueError in either of these cases (i.e., when both arguments are zero or when the computation underflows), this PR avoids checking errno when calling these trig functions. As a bonus, math.atan2() is about 4% faster.

Issue: ValueError for math.atan2(0.0, 0.0) and cmath.phase(0.0) using icx #153144

The statement about range error in the standard should, I think, be interpreted as:

A range error occurs if (x is positive) and (y/x is nonzero) and (y/x is too close to zero).

bedevere-app · 2026-07-05T19:19:11Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

hpkfft · 2026-07-06T23:23:59Z

If this PR is merged, it looks to me like #146402 can (and should) be reverted.

skirpichev

No, I doubt that this is a right approach. We must handle error in same way for all libm functions.

Perhaps, m_atan2() (like m_log1p() we have currently) could be restored (see #122681) to workaround broken platform functions.

hpkfft · 2026-07-07T04:45:51Z

We must handle error in same way for all libm functions.

The mathmodule currently has 3 ways of handling 1-argument libm functions:

Functions that cannot overflow (e.g., acos, atan) use math_1(arg, func, /*can_overflow=*/0, err_msg)
Functions that can overflow (e.g., cosh, exp) use math_1(arg, func, /*can_overflow=*/1, err_msg)
Functions that are known to set errno properly (e.g., erf, gamma) use math_1a(arg, func, err_msg)

Maybe it's not so bad to have math_2ne for 2-argument functions that are known not to need error checking (e.g., atan2) as well as the current math_2 (for the others).

skirpichev · 2026-07-08T01:01:16Z

Perhaps, we could modify math_2() helper to check that errno!=EDOM, if inputs and output are finite. When domain error occurs - result should be nan.

But I think that a little wrapper for libm's atan2 is better, if we are going to add some workaround for the given issue.

skirpichev

Please revert unrelated changes.

hpkfft · 2026-08-13T22:38:31Z

@vstinner, can you look at this at your convenience?
I anticipate that this PR will also fix Solaris to return the correct results (#138573), so the relevant math/cmath tests will not need to be skipped on that platform. Please advise whether I should revert #146402 as part of this PR. If so, I'll do that and would then ask you to run on the buildbots.

vstinner

If this PR is merged, it looks to me like #146402 can (and should) be reverted.

I would prefer to update tests for Solaris in the same PR.

vstinner · 2026-08-14T11:17:33Z

    double phi;

-    errno = 0;
    phi = atan2(z.imag, z.real); /* should not cause any exception */


Please add comment: // gh-153144: ignore atan2() errno on purpose.

vstinner · 2026-08-14T11:18:10Z

+    if (y == -1.0 && PyErr_Occurred()) {
+        return NULL;
+    }
+    r = (*func)(x, y);


Please add comment: // ignore errno on purpose.

vstinner · 2026-08-14T11:18:57Z

      "Return the arc tangent (measured in radians) of x.\n\n"
      "The result is between -pi/2 and pi/2.")
-FUNC2(atan2, atan2,
+FUNC2NE(atan2, atan2,


Please add comment: // gh-153144: Ignore atan2() and atan2pi() errno on purpose.

vstinner · 2026-08-14T11:21:51Z

-    errno = 0;
    phi = atan2(z.imag, z.real); /* should not cause any exception */
-    if (errno != 0)
-        return math_error();


I expected a test_cmath failure when this code path is removed. Is it because glibc math library doesn't errno in this case?

Yes. With the exception of the Intel and Solaris math libraries, errno is not set in this case by any math library that Python cares about. I say this because test_phase in Lib/test/test_cmath.py has asserted that return values are correct since Python 3.14, and nobody has complained. If errno were set by the C math library, the unittest would fail with ValueError: math domain error.

🌱 Some math libraries (e.g., musl) don't set errno for anything, so Python cannot rely on errno for detecting overflow or invalid. I would think that errno checking can be removed everywhere....

This reverts commit 8e1469c.

skirpichev · 2026-08-30T03:44:41Z

You also could use new helper function for copysign.

On another hand, I would prefer just remove errno stuff from math_2(), per #156145. It should be possible for all two-argument functions: atan2, atan2pi, copysign, remainder and fmod (which could utilize same wrapper).

skirpichev · 2026-08-30T03:45:33Z

-        return math_error();
-    else
-        return PyFloat_FromDouble(phi);
+    /* gh-153144: Ignore atan2() errno on purpose. */


IMO, it's rather redundant after inline comment above.

Victor asked for all these comments.

skirpichev · 2026-08-30T03:46:13Z

      "Return the arc tangent (measured in radians) of x.\n\n"
      "The result is between -pi/2 and pi/2.")
-FUNC2(atan2, atan2,
+FUNC2NE(atan2, atan2,  // gh-153144: Ignore atan2() errno on purpose.


Suggested change

FUNC2NE(atan2, atan2, // gh-153144: Ignore atan2() errno on purpose.

FUNC2NE(atan2, atan2,

skirpichev · 2026-08-30T03:46:27Z

      "Return the arc tangent (measured in radians) of y/x.\n\n"
      "Unlike atan(y/x), the signs of both x and y are considered.")
-FUNC2(atan2pi, m_atan2pi,
+FUNC2NE(atan2pi, m_atan2pi,  // gh-153144: Ignore atan2pi() errno on purpose.


Suggested change

FUNC2NE(atan2pi, m_atan2pi, // gh-153144: Ignore atan2pi() errno on purpose.

FUNC2NE(atan2pi, m_atan2pi,

skirpichev · 2026-08-30T03:47:52Z

+    if (y == -1.0 && PyErr_Occurred()) {
+        return NULL;
+    }
+    r = (*func)(x, y);  // Ignore errno on purpose.


Perhaps, expand the comment for function?

hpkfft · 2026-08-30T04:27:46Z

You also could use new helper function for copysign.

My thinking is only to do what is necessary to fix the bug (the incorrect results when building with Intel or Solaris math libraries). Then, I would recommend applying this fix to Python 3.14 and 3.15. With that in mind, I don't want to make any enhancements that are not necessary to fix the reported bug.

On another hand, I would prefer just remove errno stuff from math_2(), per #156145. It should be possible for all two-argument functions: atan2, atan2pi, copysign, remainder and fmod (which could utilize same wrapper).

Yes, 156145 can have a more ambitious goal.

Can you trigger the buildbots to run this PR? I am not able to check myself that it works on Solaris. (I do think it will work based on what I read in the Solaris bug report.)

skirpichev · 2026-08-30T05:02:12Z

My thinking is only to do what is necessary to fix the bug

Then we loose chance of using new helper function for copysign().

Can you trigger the buildbots to run this PR?

!buildbot Solaris

skirpichev · 2026-08-30T05:06:29Z

!buildbot Solaris

bedevere-bot · 2026-08-30T05:06:33Z

🤖 New build scheduled with the buildbot fleet by @skirpichev for commit 5b6d005 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F153148%2Fmerge

The command will test the builders whose names match following regular expression: Solaris

The builders matched are:

SPARCv9 Oracle Solaris 11.4 PR

hpkfft · 2026-08-30T05:19:57Z

Then we loose chance of using new helper function for copysign().

OK, I'll add that commit tomorrow. Since it will be a separate commit, it can easily be reverted (or skipped when squashing all the commits before the final merge). The same is true for the comments Victor requested--they're in their own separate commit.

pythongh-153144: Avoid checking errno for atan2

503fd4a

bedevere-app Bot mentioned this pull request Jul 5, 2026

ValueError for math.atan2(0.0, 0.0) and cmath.phase(0.0) using icx #153144

Open

bedevere-app Bot added the awaiting review label Jul 5, 2026

picnixz requested a review from skirpichev July 6, 2026 23:41

skirpichev reviewed Jul 7, 2026

View reviewed changes

pythongh-153144: Added NEWS entry

c9c51ad

skirpichev requested changes Jul 20, 2026

View reviewed changes

Comment thread Modules/mathmodule.c Outdated

Comment thread Modules/mathmodule.c Outdated

bedevere-app Bot added awaiting core review and removed awaiting review labels Jul 20, 2026

hpkfft added 3 commits July 21, 2026 07:59

pythongh-153144: Modify comments as requested in review

287e6ee

Merge branch 'main' into atan2

cbe759a

Merge branch 'main' into atan2

91257f2

vstinner reviewed Aug 14, 2026

View reviewed changes

hpkfft and others added 3 commits August 14, 2026 08:57

Add comments as requested in review

25ec8e2

Revert pythongh-146402 and update NEWS entry

43f7398

This reverts commit 8e1469c.

Merge branch 'main' into atan2

5b6d005

skirpichev reviewed Aug 30, 2026

View reviewed changes

	FUNC2NE(atan2, atan2, // gh-153144: Ignore atan2() errno on purpose.
	FUNC2NE(atan2, atan2,

	FUNC2NE(atan2pi, m_atan2pi, // gh-153144: Ignore atan2pi() errno on purpose.
	FUNC2NE(atan2pi, m_atan2pi,

Uh oh!

Uh oh!

Conversation

hpkfft commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app Bot commented Jul 5, 2026

Uh oh!

hpkfft commented Jul 6, 2026

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

hpkfft commented Jul 7, 2026

Uh oh!

skirpichev commented Jul 8, 2026

Uh oh!

skirpichev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hpkfft commented Aug 13, 2026

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skirpichev commented Aug 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hpkfft Aug 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hpkfft commented Aug 30, 2026

Uh oh!

skirpichev commented Aug 30, 2026

Uh oh!

skirpichev commented Aug 30, 2026

Uh oh!

bedevere-bot commented Aug 30, 2026

Uh oh!

hpkfft commented Aug 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hpkfft commented Jul 5, 2026 •

edited

Loading

hpkfft Aug 30, 2026 •

edited

Loading