float-cast-overflow: converting NaN to integers in numpy C code triggered by pandas

[vonosmas]

Converting floating-point value NaN to any integer data type is an undefined behavior in C. However, it actually happens in numpy extension module, which is probably caused by incorrect usage of it from pandas. If the former is built with Clang+UBSan (http://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html), there are error reports indicating the issue.

Now, this is somewhat tricky to reproduce, because it involves building NumPy C code with UBSan. The following instructions should work on Ubuntu 14.04

• Get fresh enough Clang (e.g. 3.8.0rc2 from http://llvm.org/pre-releases/3.8.0/rc2)
• Build NumPy with Clang and float-cast-overflow detection

  git clone git://github.com/numpy/numpy.git
  cd numpy
  CC=clang CXX=clang++ LDSHARED=clang CFLAGS="-fsanitize=float-cast-overflow" python setup.py install

• Fetch latest pandas
• Export ASan runtime library to provide UBSan implementation, setup runtime flags for sanitizers:

export ASAN_OPTIONS=detect_leaks=0
export UBSAN_OPTIONS=print_stacktrace=1
export LD_PRELOAD=/lib/clang/3.9.0/lib/linux/libclang_rt.asan-x86_64.so 

• Build pandas

cd pandas
python setup.py build_ext --inplace
python setup.py install

• Run tests from the test suite triggering the issue

nosetests pandas/tests/test_groupby.py:TestGroupBy.test_agg_nested_dicts
numpy/core/src/multiarray/lowlevel_strided_loops.c.src:865:17: runtime error: value nan is outside the range of representable values of type 'long'
    #0 0x7fc581bffe41 in _cast_double_to_long /usr/local/google/numpy/numpy/core/src/multiarray/lowlevel_strided_loops.c.src:865:17
    #1 0x7fc581b7faf7 in raw_array_assign_array /usr/local/google/numpy/numpy/core/src/multiarray/array_assign_array.c:96:9
    #2 0x7fc581b80355 in PyArray_AssignArray /usr/local/google/numpy/numpy/core/src/multiarray/array_assign_array.c:351:13
    #3 0x7fc581c19a80 in array_astype /usr/local/google/numpy/numpy/core/src/multiarray/methods.c:832:13
    #4 0x49968c in PyEval_EvalFrameEx (/usr/bin/python2.7+0x49968c)

nosetests pandas/tseries/tests/test_resample.py:TestResample.test_custom_grouper
numpy/core/src/multiarray/lowlevel_strided_loops.c.src:867:22: runtime error: value nan is outside the range of representable values of type 'long'
    #0 0x7f04ba194da1 in _aligned_cast_double_to_long /usr/local/google/numpy/numpy/core/src/multiarray/lowlevel_strided_loops.c.src:867:22
    #1 0x7f04ba13d5d2 in PyArray_CastRawArrays /usr/local/google/numpy/numpy/core/src/multiarray/dtype_transfer.c:3843:5
    #2 0x7f04ba11485f in PyArray_AssignRawScalar /usr/local/google/numpy/numpy/core/src/multiarray/array_assign_scalar.c:248:13
    #3 0x7f04ba11f28a in PyArray_FillWithScalar /usr/local/google/numpy/numpy/core/src/multiarray/convert.c:464:19
    #4 0x7f04ba1af2a0 in array_fill /usr/local/google/numpy/numpy/core/src/multiarray/methods.c:150:9
    #5 0x49968c in PyEval_EvalFrameEx (/usr/bin/python2.7+0x49968c)

It seems that pandas are using "np.nan" to aggressively: by calling astype(<integer_type>) on arrays that can contain NaNs, and in calling fill(np.nan) on np.empty() arrays of integral types.

[jreback]

can you show an example w/o all of the custom builds. Just stock numpy and latest pandas (or a version).

[vonosmas]

No: you need to recompile C code in NumPy for the error to be reported (otherwise NaN is somehow silently converted to a long under the hood when you're running pandas/tests/test_groupby.py:TestGroupBy.test_agg_nested_dicts. I can try to pinpoint the exact place in pandas source code which causes this behavior.

[jreback]

so what exactly is the problem, are these crashing for you?

[vonosmas]

They are crashing in a custom setup, where I build everything from source with Clang and -fsanitize=float-cast-overflow enabled. I understand it's not the way people usually numpy/pandas (i.e. not what you get from easy_install). However, the bug report shows an actual undefined behavior, and the result of casting NaN to int can be different under different compilers/versions, and is known to be different on different architectures.

[jreback]

well you shouldn't ever use easy_install, that is really old, at the very least use pip or much better is using conda.

In any event would need an actual reproducible example on a stock install. As these tests work just fine at a glance. maybe I am missing something here.

[jreback]

if you can provide a reproducible example that demonstrates a failure on stock numpy and pandas master, pls reopen.

[vonosmas]

Your call - I totally understand it's hard to deal with the "issues" that are exposed in non-standard build configurations.

However, undefined behavior is tricky and dangerous - and if we can reliably detect it, it's worth fixing it sooner rather than later, when compilers gets smarter and will break your code in unexpected ways.

I'd argue that converting NaN to integer is never correct, and usually indicates an error in logic - even if the code currently generated in the stock builds happens to work for all the use cases - this is happening by accident. I've installed faulthandler to collect the stack trace of pandas triggering the issue:

File "/pandas/pandas/core/common.py", line 1362 in _possibly_downcast_to_dtype
File "/pandas/pandas/core/groupby.py", line 715 in _try_cast
File "/pandas/pandas/core/groupby.py", line 748 in _cython_agg_general
File "/pandas/pandas/core/groupby.py", line 996 in var
File "/pandas/pandas/core/groupby.py", line 979 in std
File "/pandas/pandas/core/groupby.py", line 2526 in aggregate
File "/pandas/pandas/core/groupby.py", line 2595 in _aggregate_multiple_funcs
File "/pandas/pandas/core/groupby.py", line 2529 in aggregate
File "/pandas/pandas/core/groupby.py", line 2595 in _aggregate_multiple_funcs
File "/pandas/pandas/core/groupby.py", line 2529 in aggregate
File "/pandas/pandas/core/base.py", line 475 in _agg_1dim
File "/pandas/pandas/core/base.py", line 492 in _agg
File "/pandas/pandas/core/base.py", line 501 in _aggregate
File "/pandas/pandas/core/groupby.py", line 3052 in aggregate
File "/pandas/pandas/core/groupby.py", line 3535 in aggregate

So, it seems that _possibly_downcast_to_dtype calls np.allclose on NaN, and NaN converted to integral data type. I'd argue that NaN should be special-cased here: as converting NaN to int gives unpredictable results.

If you don't want to deal with these issues now (while everything "works") - fine, let's just leave the bug. If you want to - I can provide help with building/testing code or patches against UB.

[jreback]

@vonosmas you would have to show what this is being called. np.allclose in that function is NEVER called with anything but bool/int dtype.

As I said, need a reproducible example.

[vonosmas]

dtype is integral, but result is not:

$ git diff pandas/core/common.py
--- a/pandas/core/common.py
+++ b/pandas/core/common.py
@@ -1359,6 +1359,7 @@ def _possibly_downcast_to_dtype(result, dtype):
             # do a test on the first element, if it fails then we are done
             r = result.ravel()
             arr = np.array([r[0]])
+            print arr, dtype
             if not np.allclose(arr, trans(arr).astype(dtype)):
                 return result
$ cat test.py
import numpy as np
from pandas import DataFrame

if __name__ == '__main__':
  df = DataFrame({'A': ['foo', 'bar', 'foo', 'bar',
                        'foo', 'bar', 'foo', 'foo'],
                  'B': ['one', 'one', 'two', 'two',
                        'two', 'two', 'one', 'two'],
                  'C': np.random.randn(8) + 1.0,
                  'D': np.arange(8)})

  g = df.groupby(['A', 'B'])

  result = g.agg({'C': {'ra': ['mean', 'std']},
                  'D': {'rb': ['mean', 'std']}})
$ python test.py
[ 1.] int64
[ nan] int64

[jreback]

This should patch it.

diff --git a/pandas/core/common.py b/pandas/core/common.py
index 6165297..dee566a 100644
--- a/pandas/core/common.py
+++ b/pandas/core/common.py
@@ -1359,7 +1359,8 @@ def _possibly_downcast_to_dtype(result, dtype):
             # do a test on the first element, if it fails then we are done
             r = result.ravel()
             arr = np.array([r[0]])
-            if not np.allclose(arr, trans(arr).astype(dtype)):
+            if isnull(arr).any() or not np.allclose(arr,
+                                                    trans(arr).astype(dtype)):
                 return result

             # a comparable, e.g. a Decimal may slip in here