Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-131032: Fix math.fma(x, y, z) zero sign#131134
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Conversation
vstinner commented Mar 12, 2025 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
Fix result sign when z is zero. Co-Authored-by: Sergey B Kirpichev <[email protected]>
skirpichev commented Mar 12, 2025
!buildbot wasi |
bedevere-bot commented Mar 12, 2025
🤖 New build scheduled with the buildbot fleet by @skirpichev for commit c95e6f9 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
bedevere-bot commented Mar 12, 2025
🤖 New build scheduled with the buildbot fleet by @vstinner for commit c95e6f9 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
vstinner commented Mar 12, 2025
Nice, test_math pass on WASI buildbots. |
vstinner commented Mar 12, 2025
Uh oh!
There was an error while loading. Please reload this page.
vstinner commented Mar 12, 2025
test_fma_zero_result() failed on Android and FreeBSD.
Unrelated failure: test_bigmem failed.
Unrelated failure: |
vstinner commented Mar 12, 2025
Test failing on Android and FreeBSD: # Corner case where rounding the multiplication would# give the wrong result.x=float.fromhex('0x1p-500') y=float.fromhex('0x1p-550') z=float.fromhex('0x1p-1000') self.assertIsNegativeZero(math.fma(x-y, x+y, -z)) self.assertIsPositiveZero(math.fma(y-x, x+y, z)) self.assertIsNegativeZero(math.fma(y-x, -(x+y), -z)) self.assertIsPositiveZero(math.fma(x-y, -(x+y), z))z is not zero in this case. |
skirpichev commented Mar 12, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Proposed patch affects only case when the last argument ( Netbsd failure also seems related to |
vstinner commented Mar 12, 2025
!buildbot wasi |
bedevere-bot commented Mar 12, 2025
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
vstinner commented Mar 12, 2025
!buildbot FreeBSD |
bedevere-bot commented Mar 12, 2025
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
vstinner commented Mar 12, 2025
!buildbot Android |
bedevere-bot commented Mar 12, 2025
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge The command will test the builders whose names match following regular expression: The builders matched are:
|
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot commented Mar 12, 2025
🤖 New build scheduled with the buildbot fleet by @vstinner for commit 4e7c5ab 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131134%2Fmerge If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again. |
serhiy-storchaka commented Mar 12, 2025
It looks this PR does not fix the real issue. It only handles one specific case To handle all cases, we need to implement |
skirpichev commented Mar 13, 2025
Well, linked issue is a real one. Though, I expected more such cases. (Maybe WASI too?) #131071 is an alternative. BTW, I think that test_fma_zero_result() could be split in that pr as well.
That's true. Only simple workarounds were considered, not implementation of fma() from scratch.
Maybe. But in this case our test suite lacks tests to trigger this. I think that at least failure with musl C stdlib may be related only to arguments with special values. |
vstinner commented Mar 13, 2025
@serhiy-storchaka is against this workaround: #131134 (comment). I abandon this approach. |
Fix result sign when z is zero.