intermittent segfault after take error

[dsm054]

Was playing around and managed to get a segfault, in several versions. It's a little intermittent, and it may have to do with how things are left after an error is encountered. The same code without the the faulty take line doesn't crash for me.

>>> import pandas as pd, sys
>>> sys.version
'2.7.3 (default, Aug  1 2012, 05:16:07) \n[GCC 4.6.3]'
>>> pd.__version__
'0.11.0.dev-14a04dd'
>>> df = pd.DataFrame({'A': {1.0: 0.0, 2.0: 6.0, 3.0: 12.0}, 'C': {1.0: 2.0, 2.0: 8.0, 3.0: 14.0}, 'B': {1.0: 1.0, 2.0: 7.0, 3.0: 13.0}})
>>> df
    A   B   C
1   0   1   2
2   6   7   8
3  12  13  14
>>> 
>>> df.take(df.unstack())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/pandas-0.11.0.dev_14a04dd-py2.7-linux-i686.egg/pandas/core/frame.py", line 2951, in take
    new_index = self.index.take(indices)
  File "/usr/local/lib/python2.7/dist-packages/pandas-0.11.0.dev_14a04dd-py2.7-linux-i686.egg/pandas/core/index.py", line 416, in take
    taken = self.view(np.ndarray).take(indexer)
IndexError: index out of range for array
>>> df.unstack()
A  2     0
   2     6
   3    12
B  2     1
   2     7
   3    13
C  2     2
   2     8
   3    14
Dtype: float64
>>> df.take(df.unstack())
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/local/lib/python2.7/dist-packages/pandas-0.11.0.dev_14a04dd-py2.7-linux-i686.egg/pandas/core/frame.py", line 2951, in take
    new_index = self.index.take(indices)
  File "/usr/local/lib/python2.7/dist-packages/pandas-0.11.0.dev_14a04dd-py2.7-linux-i686.egg/pandas/core/index.py", line 416, in take
    taken = self.view(np.ndarray).take(indexer)
IndexError: index out of range for array
>>> df.unstack()
Segmentation fault (core dumped)

[stephenwlin]

Not 100% sure, but it looks like a numpy issue:

In [1]: import pandas as pd, sys

In [2]: import numpy as np

In [3]: df=pd.DataFrame({'A': {1.0: 0.0, 2.0: 6.0, 3.0: 12.0},
   ...:                  'C': {1.0: 2.0, 2.0: 8.0, 3.0: 14.0},
   ...:                  'B': {1.0: 1.0, 2.0: 7.0, 3.0: 13.0}})

In [4]: i = df.index.view(np.ndarray)

In [5]: u = df.unstack().astype(np.int_).view(np.ndarray)

In [6]: i
Out[6]: array([1.0, 2.0, 3.0], dtype=object)

In [7]: i.take(u)
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
<ipython-input-7-8293b97d15cc> in <module>()
----> 1 i.take(u)

IndexError: index out of range for array

In [8]: i
Segmentation fault (core dumped)

Line 7 only involves numpy types, although the underlying buffers have been touched by pandas (so it's possible it's happening because pandas violated a contract somewhere). i.take doesn't seem to have any reason to affect the value of i though, since it returns a new object instead of modifying itself: I'm guessing numpy's implementation is modifying i for some optimization purpose but it wasn't coded in an exception-safe manner...leaving i in some invalid state.

Will dig in a bit more.

[dsm054]

Note that the unstack results after the first take are already off (it shouldn't be 2,2,3). On a different system I can get

>>> df.unstack()
A  1358910081     0
   2              6
   3             12
B  1358910081     1
   2              7
   3             13
C  1358910081     2
   2              8
   3             14

instead.

[stephenwlin]

Changing line 5 to u = np.asarray(df.unstack().astype(np.int_).copy()).copy() still results in a segfault as does changing line 4 to either i = np.asarray(df.index.copy()) or i = np.asarray(df.index).copy() seem to make it go away. So seems either Index is violating some contract in subclassing ndarray, ndarray.take is buggy, or both...

EDIT: actually it seems like changing 4 to either option above still segfaults (at least for me), but it requires multiple calls to ndarray.take to do so.

[stephenwlin]

@dsm054 yes, it looks like the the index contents are being affected by ndarray.take, which seems like numpy optimization gone awry (since ndarray.take doesn't semantically mutate its self argument)

[stephenwlin]

definitely numpy bug (here's with dev numpy)

In [1]: import numpy as np

In [2]: u = np.asarray([ 0, 6, 12, 1, 7, 13, 2, 8, 14])

In [3]: i = np.asarray([1.0, 2.0, 3.0]).astype(object)

In [4]: i
Out[4]: array([1.0, 2.0, 3.0], dtype=object)

In [5]: i.take(u)
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)
<ipython-input-5-8293b97d15cc> in <module>()
----> 1 i.take(u)

IndexError: index 6 is out of bounds for axis 0 with size 3

In [6]: i
Segmentation fault (core dumped)

[dsm054]

Should we close this, then, and bump it upstream?

[stephenwlin]

well, maybe pandas should work around it by checking index bounds itself first for object arrays (at some minor efficiency cost...)? in which case we might want to leave the issue open for that.

it might be important since Index arrays are usually object-based (unless specialized via a subclass)

[wesm]

why don't you replace the usage of ndarray.take in Index.take with take_nd (which should not corrupt memory)?

[stephenwlin]

yeah, that works too...thanks! (it might be marginally faster too because ndarray.take is coded for the general nd-case but we have cython specializations for most 1-d cases)

[wesm]

What's the status here?

[stephenwlin]

sorry for the lack of update, this turned out to be somewhat of a rabbit hole; basically, we're exposed to this same numpy bug everywhere where we call ndarray.take with user-supplied indices, which turns out to be a lot of places. it doesn't make that much sense to just convert one call arbitrarily so I started doing a blanket conversion...but that turns out to be the wrong thing because ndarray.take has different semantics with negative indicies than our cythonic takes (i.e. the normal ones, rather than -1 being a dummy value for fill behavior)...lots of tests were broken in subtle ways before I realized the issue and regardless it's unclear if anyone is depending on the normal semantics already (since there's no documentation either way that this is supported officially, afaict).

i think the right thing to do is probably the following:

1. audit the code and try to disambiguate uses of user-supplied indices and algorithm-generated indices throughout
2. convert the former to either a bug-free cythonic take that does normal negative indices, or a bounds-checking wrapper around ndarray.take
3. (optional) if new cythonic take is used, then get rid of the whole back and forth "platform int <-> int64" conversion mess, which is basically only there to support ndarray.take.

but that's a pretty big change...i don't have free cycles for it right now but if someone else wants to take a look they're free to

[stephenwlin]

(btw, in case someone picks this up...there's no bounds checking in take_nd in most cases, which might be ok if we can ensure that no user-supplied indices are every passed to it, but maybe it should be addressed to be safe; probably too slow to turn on cython's built-in bounds check though, since that'll check with every access, better thing to do would be to add the minimal checks manually for a row/column of calls at a time)

[jreback]

this is the change I made in _ixs in frame.py (this was the old icol), as negative user-supplied indicies were incorrect (the change was all wrapped up in the .iloc changes). I am sure as @stephenwlin points out there are more user facing changes like this (prob not too many, and most prob hit this code-block anyhow)

can anyone think of a place where there are user-facing posssibilites of negative indices except:

• _ixs/icol
• frame/take

other uses of negative indices denote labels (or hit _ixs) AFAICT