Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
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 ourterms of service andprivacy 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
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
This PR is a copy the followinghttps://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(). |
I would prefer to get the same behavior for fma(x, y, z) than
|
bedevere-bot commentedJan 13, 2020
Strange. Tests passed on Linux and Windows pre-commit CIs. Let me try on stable buildbots. |
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 commentedJan 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 |
You can also look at |
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). |
Failures on x86 Windows7 PR (32-bit):
pythoninfo:
"MSC v.1913" is Visual Studio 2017 Update 6. |
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:
(theemphasis 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? |
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. |
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. |
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 😉. |
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? ... |
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 commentedJan 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 commentedJan 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. |
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 stillmultiple 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 ;-) |
Uh oh!
There was an error while loading.Please reload this page.
Co-Authored-By: Mark Dickinsonmdickinson@enthought.com
https://bugs.python.org/issue29282