Skip to content

Conversation

skirpichev
Copy link
Contributor

@skirpichev skirpichev commented Sep 9, 2024

Just like cmath_testcases.txt. These tests require IEEE 754 anyway.

Just like cmath_testcases.txt.  These tests require IEEE 754 anyway.
@skirpichev

This comment was marked as outdated.

@skirpichev
Copy link
Contributor Author

CC @picnixz

@picnixz
Copy link
Member

picnixz commented Sep 13, 2024

Sorry Sergey for not replying to your pings. I see them but I'm either doing something else or then I forget about them :(

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vstinner vstinner merged commit 28aea5d into python:main Sep 17, 2024
33 checks passed
@vstinner
Copy link
Member

Merged, thanks.

@vstinner vstinner added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Sep 17, 2024
@miss-islington-app
Copy link

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

@miss-islington-app
Copy link

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 17, 2024
)

Just like cmath_testcases.txt. These tests require IEEE 754 anyway.

Correct zero sign for sqrt tests to match math.h convention.
(cherry picked from commit 28aea5d)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Sep 17, 2024

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 17, 2024
)

Just like cmath_testcases.txt. These tests require IEEE 754 anyway.

Correct zero sign for sqrt tests to match math.h convention.
(cherry picked from commit 28aea5d)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Sep 17, 2024
@bedevere-app
Copy link

bedevere-app bot commented Sep 17, 2024

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

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Sep 17, 2024
@skirpichev skirpichev deleted the math_testcases-szero-123836 branch September 17, 2024 08:29
vstinner pushed a commit that referenced this pull request Sep 17, 2024
…124162)

gh-123836: Check zero signs in math_testcases.txt (GH-123854)

Just like cmath_testcases.txt. These tests require IEEE 754 anyway.

Correct zero sign for sqrt tests to match math.h convention.
(cherry picked from commit 28aea5d)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Windows10 3.x has failed when building commit 28aea5d.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/146/builds/9512) and take a look at the build logs.
  4. Check if the failure is related to this commit (28aea5d) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/146/builds/9512

Failed tests:

  • test_math

Failed subtests:

  • testFmod - test.test_math.MathTests.testFmod

Summary of the results of the build (if available):

==

Click to see traceback logs
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)

@vstinner
Copy link
Member

Oh, a test failed on Windows 10:

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)

@vstinner
Copy link
Member

@skirpichev: Would you mind to have a look at the testFmod() failure?

@skirpichev
Copy link
Contributor Author

It's easy: windows=>bugs. Standard says: "The fmod functions compute the floating-point remainder of x/y. [...] if y is nonzero, the result has the same sign as x [...]".

Does it fail on 11? Maybe it's fixed?

Anyway, I'll provide patch to filter out this on Win 10.

@vstinner
Copy link
Member

Anyway, I'll provide patch to filter out this on Win 10.

Is it possible to fix fmod() bug on Windows?

@skirpichev
Copy link
Contributor Author

Unfortunately, I can only guess how it's broken.

Assuming that it's wrong only for signed zero as result, we could replace one with

static double
m_fmod(double x, double y)
{
    double r = fmod(x, y);

    if (r == 0.0 && y != 0.0) {
        r = copysign(r, x);
    }

    return r;
}

@vstinner
Copy link
Member

Does it fail on 11? Maybe it's fixed?

The test pass on [MSC v.1941 64 bit (AMD64)] (Windows 10 buildbot), but fails on [MSC v.1916 64 bit (AMD64)] (GitHub Actions). It's more a matter of the C Compiler version (MSC).

savannahostrowski pushed a commit to savannahostrowski/cpython that referenced this pull request Sep 22, 2024
Just like cmath_testcases.txt. These tests require IEEE 754 anyway.

Correct zero sign for sqrt tests to match math.h convention.
Yhg1s pushed a commit that referenced this pull request Sep 30, 2024
…124161)

gh-123836: Check zero signs in math_testcases.txt (GH-123854)

Just like cmath_testcases.txt. These tests require IEEE 754 anyway.

Correct zero sign for sqrt tests to match math.h convention.
(cherry picked from commit 28aea5d)

Co-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news tests Tests in the Lib/test dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants