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
float('nan') returns 0.0 on Python compiled with icc #65366
Comments
On a Python compiled with Intel C compiler, float('nan') returns 0.0. This behavior can be reproduced with icc versions 11 and 12. The definition of Py_NAN in include/pymath.h expects /* Py_NAN
* A value that evaluates to a NaN. On IEEE 754 platforms INF*0 or
* INF/INF works. Define Py_NO_NAN in pyconfig.h if your platform
* doesn't support NaNs.
*/
#if !defined(Py_NAN) && !defined(Py_NO_NAN)
#define Py_NAN (Py_HUGE_VAL * 0.)
#endif I don't have a copy of the standard, but a simple test shows that the above does not hold for icc. When compiled with icc without any flags, the following program outputs "inf 0.000000" instead of the expected "inf nan": #include <stdio.h>
#include <math.h>
int main()
{
double inf = HUGE_VAL;
double nan = HUGE_VAL * 0;
printf("%f %f\n", inf, nan);
return 0;
} Compiling the same program with gcc yields the expected output of "inf nan". This issue can be fixed (or worked around, if it's an icc bug) by using a different definition of Py_NAN. On icc, defining Py_NAN as The attached patch fixes the issue, taking the conservative approach of only modifying the icc definition of Py_NAN. |
Did you compile with "-fp-model strict"? IIRC icc is not IEEE-754 |
What's |
sys.float_repr_style is 'short'. I haven't actually tried this in Python 3.5, but I did note the same definition of Py_NAN (which is used elsewhere in the code), so I tagged the issue as likely also present in 3.x. |
The compilation was performed with the default flags, i.e. without -fp-model strict or similar. If Python requires strict IEEE 754 floating-point, it should probably be mentioned at a prominent place in the documentation. Administrators building Python with alternative compilers or on non-gcc Unix systems might not be aware of the issue. |
So I believe that this particular issue (float('nan') giving zero) should not be present on Python 3.4 or above. The bogus definition of Py_NAN will still cause problems in some math and cmath return values, though. |
It doesn't (at the moment). |
Mark:
Does this imply that my patch is a better fix than requiring the builder to specify -fp-model strict to icc? For our use case either solution is valid. For administrators and users testing or using Python with icc, it might be more convenient for Py_NAN to do the right thing with the default icc flags. |
Sorry; that was an unhelpful kneejerk reply. There are two aspects to this. (1) Currently, at least in theory, CPython doesn't require IEEE 754 *at all*: in theory, it should work with whatever floating-point format the platform's "double" type happens to provide. In practice I suspect there's a lot more IEEE 754 dependence in the codebase than we realize, and I'd expect to see a fair amount of test breaking if Python were ever to meet a non-IEEE 754 platform. (Reports of any *recent* occurrences of Python in the wild meeting non-IEEE 754 platforms would be welcomed.) (2) If CPython *does* figure out that it's running on an IEEE 754 platform then yes, care should be taken with compiler flags to ensure that we're not using compiler optimisations that have the potential to break IEEE semantics. Without this there's a significant danger of float to string conversions getting messed up; there are also a few odds and ends in the math and cmath libraries that depend on careful IEEE semantics. (The implementations of fsum and log1p, for example.) I think the right fix here is to add the relevant compiler flag to the build. That said, I'd *also* love to see remaining uses of Py_NAN fixed to use _Py_dg_stdnan where available. |
I agree we should add "-fp-model strict" to the icc build flags, provided that Apparently icc users want unsafe fast math by default, so this flag should |
Using -fp-model strict (or other appropriate icc flag) seems like a reasonable resolution. It should likely also be applied to Python 3.x, despite the version field of this issue. (Even if float('nan') happens to work in current 3.x, internal code that returns Py_NAN can and will break.) |
Mark, if you agree that "fp-model strict" should not show up in |
Sure, if that's really the expectation. I'm not sure I'd want any of *my* extension modules being built with non-strict FP, but there's a bit of a personal bias there. |
I quite agree, and it's hard to tell what users want. Basically I'm afraid I would find either decision reasonable. |
Note that defaulting to unsafe math in extensions will make *their* use of the Py_NAN macro break under icc. If we go that route ("-fp-model strict" for Python build, but not for extensions), we should also apply the attached patch that defines Py_NAN as Py_HUGE_VAL / Py_HUGE_VAL. |
HI, icc builds of python 2.7 seem to have issues handling nan, inf, etc $ /usr/local/python-2.7.6/bin/python
Python 2.7.6 (default, Jan 10 2014, 12:14:02)
[GCC Intel(R) C++ gcc 4.1 mode] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> print float('nan')
0.0
$ /usr/local/python-2.6.6-64bit/bin/python
Python 2.6.6 (r266:84292, Oct 14 2010, 15:47:19)
[GCC Intel(R) C++ gcc 4.1 mode] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> print float('nan')
nan I tried both –fp-model strict and –fp-model precise compiler options as suggested by http://bugs.python.org/issue21167, but neither seems to resolve other situations like the one with atan2 below: $ LD_LIBRARY_PATH=/dat/sharefolder_scratch/python-build ./python
Python 2.7.9 (default, Feb 18 2015, 19:58:37)
[GCC Intel(R) C++ gcc 4.1 mode] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> print float('nan')
nan
>>> import math
>>> print math.atan2(0, float('nan'))
0.0
$ /usr/local/python-2.6.6-64bit/bin/python
Python 2.6.6 (r266:84292, Oct 14 2010, 15:47:19)
[GCC Intel(R) C++ gcc 4.1 mode] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import math
>>> print math.atan2(0, float('nan'))
nan |
please disregard , the issue is resolved with your patch. |
Producing NaN by Py_HUGE_VAL / Py_HUGE_VAL as in the suggested patch is unsafe as it can generate a FP exception during runtime. Also aggresive compiler FP optimizations can eliminate this calculation on compile-time. For this reason, we've used constant referencing in our fix, which will work regardless of how -fp-model is set. The problem is that the compiler is free to pre-compute the result as both values in 0*Inf are constants. An aggressively optimizing compiler could treat 0 * x = 0 no matter what x is. So under aggressive floating point optimizations setting we could get a wrong value, and that is indeed what happens. Another problem is that 0 * Inf along with resulting in a QNaN value should raise an invalid floating point exception flag. If the result is pre-computed at compile time, then the user won’t see the flag (it is another question whether the user wanted the flag or not originally). Our patch preserves both the value and the side effect, and leaves people free to build with the flags they want. |
Note that Chris' patch is coming from Intel. (The ICC buildbots are currently building with -fp-model strict, by the way.) Mark, Stefan, what do you think? Is this a good idea? IIUC we would then not have to worry about differentiating between the python build and the distutils flags, and users would get whatever they specified at python build time. |
Looks fine to me. IIRC, we moved the PyFloat_FromString implementation away from using Py_NAN in Python 3 for exactly this reason. On this point, though:
Wouldn't such a compiler be in violation of the C standard, at least if it defines __STDC_IEC_559__? |
From Clark Nelson:
|
I've applied this patch to 2.7 on OSX and compiled without -fp-model strict, and all of the tests now pass. |
Now, what's the equivalent patch for python3? Should I open a new issue for that? |
Nevermind, I forgot to try and see if it applied...and it does :) |
New changeset d9c85b6bab3a by R David Murray in branch '2.7': New changeset 5e71a489f01d by R David Murray in branch '3.4': New changeset e3008318f76b by R David Murray in branch '3.5': New changeset 1dd4f473c627 by R David Murray in branch 'default': |
Thanks Chris, and Mark. I ran the tests on 3.6 both on Linux (non ICC) and on Mac (with ICC without -fp-model strict) and all the tests passed. |
Larry, do you want this for 3.5.0a2? It's an innocuous patch for anyone not using ICC, and makes ICC "just work" (with the default ICC build arguments) for people using ICC. (Well, on (lin/u)nux and mac, anyway, I'm not sure we've resolved all the ICC issues on Windows yet.) |
Assuming that "ICC_NAN_STRICT" is only on for Intel icc: yes, please. |
Pull request accepted and merged. |
New changeset d93bb3d636cf by R David Murray in branch '3.5': New changeset c1523d5dee3c by Larry Hastings in branch '3.5': |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: