Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
bpo-29282: Add math.fma(): fused multiply-add function#17987
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 Jan 13, 2020 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
vstinner commented Jan 13, 2020
This PR is a copy the following https://hg.python.org/cpython/rev/b33012ef1417 written by Mark Dickinson. This change has been reverted, see: https://bugs.python.org/issue29282#msg285956 I created this PR to reconsider math.fma() and to use this PR as a starting point to fix the implementation. I'm now waiting the CIs to see how things are going on. If tests continue to fail on some platforms, I plan to manually handle NaN and INF in the C code, before calling libc fma(). |
vstinner commented Jan 13, 2020
I would prefer to get the same behavior for fma(x, y, z) than cc @mdickinson |
bedevere-bot commented Jan 13, 2020
vstinner commented Jan 13, 2020
Strange. Tests passed on Linux and Windows pre-commit CIs. Let me try on stable buildbots. |
mdickinson commented Jan 13, 2020
fma is a standard IEEE 754 function, so yes, the behaviour in corner cases is fully specified by IEEE 754, and we should expect to follow that behaviour. |
mdickinson commented Jan 13, 2020 • 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.
This is the usual rule about propagating NaNs, together with Python's rules for wrapping existing functions (as articulated at the top of Think of |
mdickinson commented Jan 13, 2020
You can also look at |
vstinner commented Jan 13, 2020
test_fma_zero_result() fails on FreeBSD:
Shared pythoninfo: Non-debug pythoninfo: Note: PPC64 Fedora PR failure is unrelated (test_distutils: https://bugs.python.org/issue39248). |
vstinner commented Jan 13, 2020
Failures on x86 Windows7 PR (32-bit): pythoninfo: "MSC v.1913" is Visual Studio 2017 Update 6. |
mdickinson commented Jan 13, 2020
Sorry, I'm wrong: "fully" is incorrect. There's one case where both IEEE 754 and C99 refuse to specify the behaviour, namely
while C99 says in Annex F, 9.10.1:
(the emphasis above is mine). So for us, My inclination would be to specify this in Python, and have Python return a NaN in this case, and not raise. @tim-one thoughts? |
mdickinson commented Jan 13, 2020
Okay, so we still have the same issue as last time I tried this. :-( I'm really not comfortable with knowingly delivering a substandard fma on a mainstream platform. |
tim-one commented Jan 13, 2020
Yes, just return a NaN. About the failure of single-rounding on Windows. it's surprising, and a few minutes on Google only found complaints about this in the BPO issue report. I wonder whether we're using (or failing to use) some goofy Windows-specific compiler (linker?) flag(s), but can't make time now to thrash with that. |
tim-one commented Jan 13, 2020
Is that the only Windows build with a failure? Windows 7 end-of-life is tomorrow (14 Jan 2020), and nobody cares about 32-bit boxes anymore anyway 😉. |
tim-one commented Jan 14, 2020
I checked out this branch and tried it on my own Windows box (Win 10 Pro, Visual Studo 2017). test_math passed under all 4 combinations of{Release, Debug} x{64-bit, 32-bit}. Here's a sample ID line:
So the Win32 buildbot failure may be spurious. No idea why. Ancient OS? cygwin mucking with something? Slightly earlier build of Visual Studio? The buildbot is actually running on a 32-bit box? ... |
tim-one commented Jan 14, 2020
Ah! I see now that the other Windows buildbots passed (as did my own Windows box), including an AMD64 Windows 7 SP1 bot. So there's "something wrong" with the sole Windows oddball that failed. But I don't know how to pursue that. |
mdickinson commented Jan 14, 2020 • 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.
I wish that were true, but interactions with customers say otherwise. :-( OTOH, none of those customers are going to be using Python 3.9 any time soon (one of them "upgraded" last year from Python 2 to Python 3.4). |
mdickinson commented Jan 14, 2020 • 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.
My suspicion is that use of the x87 FPU plays some role in this. |
vstinner commented Mar 10, 2020
I'm sorry but I give up on this PR. I created it to check if the CI pass on all platforms and the result is that: no, it doesn't. There are still multiple platform specific issues. I don't know how to fix these issues and I don't really need this function, so I'm not motivated to fix issues, sorry. If anyone wants to work on this, just copy/clone my PR ;-) |
Co-Authored-By: Mark Dickinson mdickinson@enthought.com
https://bugs.python.org/issue29282