float('nan') returns 0.0 on Python compiled with icc

[hniksic]

BPO	21167
Nosy	@mdickinson, @larryhastings, @hniksic, @bitdancer
Dependencies	bpo-21121: -Werror=declaration-after-statement is added even for extension modules through setup.py
Files	intel-nan.diff: Patch implementing alternative NaN definition under icc intel-nan-safe.patch: Patch for safe NaN with FP optimizations

^{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:

assignee = None
closed_at = <Date 2015-08-25.01:15:40.437>
created_at = <Date 2014-04-07.10:02:32.170>
labels = ['interpreter-core', 'type-bug']
title = "float('nan') returns 0.0 on Python compiled with icc"
updated_at = <Date 2015-08-25.21:35:25.618>
user = 'https://github.com/hniksic'

bugs.python.org fields:

activity = <Date 2015-08-25.21:35:25.618>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2015-08-25.01:15:40.437>
closer = 'r.david.murray'
components = ['Interpreter Core']
creation = <Date 2014-04-07.10:02:32.170>
creator = 'hniksic'
dependencies = ['21121']
files = ['34746', '40162']
hgrepos = []
issue_num = 21167
keywords = ['patch']
message_count = 31.0
messages = ['215687', '215689', '215692', '215698', '215700', '215701', '215703', '215705', '215706', '215707', '216454', '216468', '216583', '216584', '219027', '236244', '236245', '248407', '248410', '248424', '248469', '248478', '248483', '248484', '248519', '248520', '248521', '248549', '248666', '249079', '249164']
nosy_count = 8.0
nosy_names = ['mark.dickinson', 'larry', 'hniksic', 'r.david.murray', 'python-dev', 'jbradaric', 'Scholes.C', 'christopher.hogan']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21167'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6']

[hniksic]

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 HUGE_VAL * 0.0 to compile to a NaN value on IEEE754 platforms:

/* 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 Py_HUGE_VAL / Py_HUGE_VAL produces the intended effect. If this is considered suboptimal on other compilers, the definition can be conditionally compiled for icc only.

The attached patch fixes the issue, taking the conservative approach of only modifying the icc definition of Py_NAN.

[skrah]

Did you compile with "-fp-model strict"? IIRC icc is not IEEE-754
compliant by default.

[mdickinson]

What's sys.float_repr_style for this build? For Python 3.5 and short float repr, float('nan') shouldn't even be using Py_NAN. It should land in _Py_parse_inf_or_nan in pystrtod.c, which uses _Py_dg_stdnan to get the NaN value directly from a suitable bitpattern.

[hniksic]

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.

[hniksic]

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.

[mdickinson]

I tagged the issue as likely also present in 3.x.

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.

[mdickinson]

If Python requires strict IEEE 754 floating-point,

It doesn't (at the moment).

[hniksic]

Mark:

> If Python requires strict IEEE 754 floating-point,

It doesn't (at the moment).

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.

[mdickinson]

It doesn't (at the moment).

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.

[skrah]

I agree we should add "-fp-model strict" to the icc build flags, provided that
there is a way that it does not show up in the distutils flags.

Apparently icc users want unsafe fast math by default, so this flag should
probably not be enforced in extension module builds.

[hniksic]

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.)

[skrah]

Mark, if you agree that "fp-model strict" should not show up in
the distutils CFLAGS once Python is installed, the issue now depends
on bpo-21121.

[mdickinson]

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.