Perfect your code
With built-in code review tools, GitHub makes it easy to raise the quality bar before you ship. Join the 40 million developers who've merged over 200 million pull requests.
Sign up for free See pricing for teams and enterprisesbpo-29282: Add math.fma(): fused multiply-add function #17987
Conversation
Co-Authored-By: Mark Dickinson <mdickinson@enthought.com>
This comment has been minimized.
This comment has been minimized.
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(). |
This comment has been minimized.
This comment has been minimized.
I would prefer to get the same behavior for fma(x, y, z) than
cc @mdickinson |
This comment has been minimized.
This comment has been minimized.
bedevere-bot
commented
Jan 13, 2020
This comment has been minimized.
This comment has been minimized.
Strange. Tests passed on Linux and Windows pre-commit CIs. Let me try on stable buildbots. |
This comment has been minimized.
This comment has been minimized.
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 comment has been minimized.
This comment has been minimized.
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 |
This comment has been minimized.
This comment has been minimized.
You can also look at |
This comment has been minimized.
This comment has been minimized.
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). |
This comment has been minimized.
This comment has been minimized.
Failures on x86 Windows7 PR (32-bit):
pythoninfo:
"MSC v.1913" is Visual Studio 2017 Update 6. |
This comment has been minimized.
This comment has been minimized.
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? |
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
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 |
This comment has been minimized.
This comment has been minimized.
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? ... |
This comment has been minimized.
This comment has been minimized.
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. |
This comment has been minimized.
This comment has been minimized.
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). |
This comment has been minimized.
This comment has been minimized.
My suspicion is that use of the x87 FPU plays some role in this. |
vstinner commentedJan 13, 2020
•
edited by bedevere-bot
Co-Authored-By: Mark Dickinson mdickinson@enthought.com
https://bugs.python.org/issue29282