Skip to content

int64 overflow/wrap around with nansum() #163

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

Closed
xflr6 opened this issue Feb 18, 2017 · 5 comments
Closed

int64 overflow/wrap around with nansum() #163

xflr6 opened this issue Feb 18, 2017 · 5 comments

Comments

@xflr6
Copy link

xflr6 commented Feb 18, 2017

See pandas-dev/pandas#15453

In [1]: import bottleneck as bn

In [2]: bn.__version__
Out[2]: '1.2.0'

In [3]: import numpy as np

In [4]: bn.nansum(np.array([2**31],dtype='int64'))
Out[4]: -2147483648
@kwgoodman
Copy link
Collaborator

That's odd. On linux with py2.7:

In [1]: bn.nansum(np.array([2**31],dtype='int64'))
Out[1]: 2147483648
In [2]: bn.nansum(np.array([2**131],dtype='int64'))
OverflowError: Python int too large to convert to C long

Python, not bottleneck, raises on the second example.

@cgohlke
Copy link
Contributor

cgohlke commented May 4, 2017

Using PyLong_FromLongLong instead of PyInt_FromLong in reduce_template.c fixes this issue as well as pandas-dev/pandas#15453 for me on Windows.

PyLong_FromLongLong is probably too much for the 32-bit functions, but it works here.

Would be nice to have this issue fixed in the 1.2.1 release #168.

kwgoodman added a commit that referenced this issue May 5, 2017
@cgohlke
Copy link
Contributor

cgohlke commented May 5, 2017

I was using this:

diff --git a/bottleneck/src/reduce_template.c b/bottleneck/src/reduce_template.c
index d9fa955..031e1eb 100644
--- a/bottleneck/src/reduce_template.c
+++ b/bottleneck/src/reduce_template.c
@@ -129,7 +129,7 @@ REDUCE_ALL(nansum, DTYPE0)
         NEXT
     }
     BN_END_ALLOW_THREADS
-    return PyInt_FromLong(asum);
+    return PyLong_FromLongLong(asum);
 }
 
 REDUCE_ONE(nansum, DTYPE0)
@@ -506,7 +506,7 @@ REDUCE_ALL(NAME, DTYPE0)
         NEXT
     }
     BN_END_ALLOW_THREADS
-    return PyInt_FromLong(extreme);
+    return PyLong_FromLongLong(extreme);
 }
 
 REDUCE_ONE(NAME, DTYPE0)
@@ -571,7 +571,7 @@ REDUCE_ALL(NAME, DTYPE0)
         VALUE_ERR("All-NaN slice encountered");
         return NULL;
     } else {
-        return PyInt_FromLong(idx);
+        return PyLong_FromLongLong(idx);
     }
 }
 
@@ -636,7 +636,7 @@ REDUCE_ALL(NAME, DTYPE0)
     }
     BN_END_ALLOW_THREADS
     DECREF_INIT_ALL_RAVEL
-    return PyInt_FromLong(idx);
+    return PyLong_FromLongLong(idx);
 }
 
 REDUCE_ONE(NAME, DTYPE0)
@@ -728,7 +728,7 @@ REDUCE_ALL(ss, DTYPE0)
         NEXT
     }
     BN_END_ALLOW_THREADS
-    return PyInt_FromLong(asum);
+    return PyLong_FromLongLong(asum);
 }
 
 REDUCE_ONE(ss, DTYPE0)

kwgoodman added a commit that referenced this issue May 5, 2017
@kwgoodman
Copy link
Collaborator

@cgohlke Your bug fix is a huge help. Thank you!

@kwgoodman
Copy link
Collaborator

@xflr6 thanks for the bug report
@cgohlke thanks for the fix

merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants