-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
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
Conversation
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
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(). |
I would prefer to get the same behavior for fma(x, y, z) than
cc @mdickinson |
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. |
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:
(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? |
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. |
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). |
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 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