gh-123836: workaround fmod(x, y) bug on Windows by skirpichev · Pull Request #124171 · python/cpython

skirpichev · 2024-09-17T12:17:05Z

Buildbot failure on Windows 10 with MSC v.1916 64 bit (AMD64):

FAIL: testFmod (test.test_math.MathTests.testFmod) ---------------------------------------------------------------------- Traceback (most recent call last):
  File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 605, in testFmod
    self.ftest('fmod(-10, 1)', math.fmod(-10, 1), -0.0)
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 258, in ftest
    self.fail("{}: {}".format(name, failure))
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: fmod(-10, 1): expected -0.0, got 0.0 (zero has wrong sign)

Here Windows loose sign of the result; if y is nonzero, the result should have the same sign as x.

This amends 28aea5d.

Issue: Testing math_testcases.txt lacks support for signed zeros #123836

Buildbot failure on Windows 10 with MSC v.1916 64 bit (AMD64): FAIL: testFmod (test.test_math.MathTests.testFmod) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 605, in testFmod self.ftest('fmod(-10, 1)', math.fmod(-10, 1), -0.0) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 258, in ftest self.fail("{}: {}".format(name, failure)) ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AssertionError: fmod(-10, 1): expected -0.0, got 0.0 (zero has wrong sign) Here Windows loose sign of the result; if y is nonzero, the result should have the same sign as x. This amends 28aea5d.

skirpichev · 2024-09-17T12:21:02Z

There are few other places in the math module, where fmod() is used. I kept library function: this bug shouldn't affect mentioned cases.

Modules/_math.h

vstinner · 2024-09-17T13:26:46Z

Modules/_math.h

+     */
+    double r = fmod(x, y);
+
+    if (r == 0.0 && y != 0.0) {


Can you make this code specific to Windows? For example, use #ifdef _MSC_VER. Move the comment on this part of the code in this case.

Do we sure that this issue specific only for Windows?

Modules/mathmodule.c

This reverts commit d3a0af6.

Modules/mathmodule.c

vstinner

LGTM

miss-islington-app · 2024-09-17T17:16:19Z

Thanks @skirpichev for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington-app · 2024-09-17T17:16:23Z

Sorry, @skirpichev and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker f4dd4402108cc005d45acd4ca83c8530c36a93ca 3.13

miss-islington-app · 2024-09-17T17:16:38Z

Sorry, @skirpichev and @vstinner, I could not cleanly backport this to 3.12 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker f4dd4402108cc005d45acd4ca83c8530c36a93ca 3.12

vstinner · 2024-09-17T17:34:59Z

@skirpichev: You closed the PR, but automated backport failed. Would you mind to try to backport manually the fix to the 3.13 branch? Maybe from there, we can automate the backport to 3.12.

bedevere-app · 2024-09-17T17:54:17Z

GH-124186 is a backport of this pull request to the 3.12 branch.

…H-124171) Buildbot failure on Windows 10 with MSC v.1916 64 bit (AMD64): FAIL: testFmod (test.test_math.MathTests.testFmod) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 605, in testFmod self.ftest('fmod(-10, 1)', math.fmod(-10, 1), -0.0) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 258, in ftest self.fail("{}: {}".format(name, failure)) ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AssertionError: fmod(-10, 1): expected -0.0, got 0.0 (zero has wrong sign) Here Windows loose sign of the result; if y is nonzero, the result should have the same sign as x. This amends commit 28aea5d. (cherry picked from commit f4dd440) Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>

bedevere-app · 2024-09-17T18:03:04Z

GH-124187 is a backport of this pull request to the 3.13 branch.

…124186) Buildbot failure on Windows 10 with MSC v.1916 64 bit (AMD64): FAIL: testFmod (test.test_math.MathTests.testFmod) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 605, in testFmod self.ftest('fmod(-10, 1)', math.fmod(-10, 1), -0.0) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 258, in ftest self.fail("{}: {}".format(name, failure)) ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AssertionError: fmod(-10, 1): expected -0.0, got 0.0 (zero has wrong sign) Here Windows loose sign of the result; if y is nonzero, the result should have the same sign as x. This amends commit 28aea5d. (cherry picked from commit f4dd440)

vstinner · 2024-09-17T19:47:06Z

Thanks for the backports.

Buildbot failure on Windows 10 with MSC v.1916 64 bit (AMD64): FAIL: testFmod (test.test_math.MathTests.testFmod) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 605, in testFmod self.ftest('fmod(-10, 1)', math.fmod(-10, 1), -0.0) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 258, in ftest self.fail("{}: {}".format(name, failure)) ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AssertionError: fmod(-10, 1): expected -0.0, got 0.0 (zero has wrong sign) Here Windows loose sign of the result; if y is nonzero, the result should have the same sign as x. This amends commit 28aea5d.

…124187) Buildbot failure on Windows 10 with MSC v.1916 64 bit (AMD64): FAIL: testFmod (test.test_math.MathTests.testFmod) ---------------------------------------------------------------------- Traceback (most recent call last): File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 605, in testFmod self.ftest('fmod(-10, 1)', math.fmod(-10, 1), -0.0) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "D:\buildarea\3.x.bolen-windows10\build\Lib\test\test_math.py", line 258, in ftest self.fail("{}: {}".format(name, failure)) ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ AssertionError: fmod(-10, 1): expected -0.0, got 0.0 (zero has wrong sign) Here Windows loose sign of the result; if y is nonzero, the result should have the same sign as x. This amends commit 28aea5d. (cherry picked from commit f4dd440)

bedevere-app bot added the awaiting review label Sep 17, 2024

bedevere-app bot mentioned this pull request Sep 17, 2024

Testing math_testcases.txt lacks support for signed zeros #123836

Closed

vstinner reviewed Sep 17, 2024

View reviewed changes

address review: drop m_fmod

49c13a6

skirpichev marked this pull request as draft September 17, 2024 14:24

bedevere-app bot removed the awaiting review label Sep 17, 2024

skirpichev added 3 commits September 17, 2024 17:25

XXX filter warnings?

d3a0af6

Revert "XXX filter warnings?"

aedcf74

This reverts commit d3a0af6.

Move workaround to math_fmod_impl

fbecd8b

skirpichev marked this pull request as ready for review September 17, 2024 14:59

bedevere-app bot added the awaiting review label Sep 17, 2024

add news

5a15cbe

skirpichev added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Sep 17, 2024

vstinner reviewed Sep 17, 2024

View reviewed changes

Modules/mathmodule.c Outdated Show resolved Hide resolved

address review: restrict fix to Windows

32aa012

vstinner approved these changes Sep 17, 2024

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Sep 17, 2024

vstinner enabled auto-merge (squash) September 17, 2024 17:14

vstinner merged commit f4dd440 into python:main Sep 17, 2024

bedevere-app bot removed the awaiting merge label Sep 17, 2024

miss-islington-app bot assigned vstinner Sep 17, 2024

skirpichev deleted the fix-fmod-win-123854 branch September 17, 2024 17:22

bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 17, 2024

bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 17, 2024

zware mentioned this pull request Sep 18, 2024

Fix make htmllive target #124219

Merged

Uh oh!

Conversation

skirpichev commented Sep 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skirpichev commented Sep 17, 2024

Uh oh!

Uh oh!

vstinner Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

skirpichev Sep 17, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington-app bot commented Sep 17, 2024

Uh oh!

miss-islington-app bot commented Sep 17, 2024

Uh oh!

miss-islington-app bot commented Sep 17, 2024

Uh oh!

vstinner commented Sep 17, 2024

Uh oh!

bedevere-app bot commented Sep 17, 2024

Uh oh!

bedevere-app bot commented Sep 17, 2024

Uh oh!

vstinner commented Sep 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

skirpichev commented Sep 17, 2024 •

edited

Loading