ENH: sparse: speed up LIL indexing + assignment via Cython #3356
Conversation
|
I think you can switch off bounds checking and wraparound for a few more functions... |
|
I didn't get measureable performance improvement out from disabling wraparound/bounds. It's also possible someone passes in wrong sized rows/data arrays, so if I add the decorators, more checks would be needed. I'd rather keep it simpler. |
|
Okay. On 20 February 2014 11:09, Pauli Virtanen notifications@github.com wrote:
|
| @@ -30,6 +30,7 @@ scipy/io/matlab/mio_utils.c binary | |||
| scipy/io/matlab/mio5_utils.c binary | |||
| scipy/io/matlab/streams.c binary | |||
| scipy/signal/_spectral.c binary | |||
| scipy/sparse/_csparsetools.c | |||
There was a problem hiding this comment.
I assume that should have gone into .gitignores?
aren't the gitattributes for the cython files are now obsolete as they are not included in the repository anymore?
There was a problem hiding this comment.
Indeed, I intended to put it to gitignore. Fixed.
|
@jnothman did you finish your review? This could still make it into 0.14.x if merged in time. |
| # Scalar fast path first | ||
| if isinstance(index, tuple) and len(index) == 2: | ||
| i, j = index | ||
| if ((isinstance(i, int) or isinstance(i, np.integer)) and |
There was a problem hiding this comment.
other implementations use sputils.isintlike which has slightly different semantics. For consistency, it should apply here.
There was a problem hiding this comment.
Using isintlike or isscalarlike slows the fast path down by 25-50%.
Hence the splitting of it into two parts.
|
Sorry for so many minor comments. Really, none should be a blocker, and this LGTM! |
As per review comments: - explain and rename prepare_index_arrays - indicate out-of-bounds column indices - faster insertion in lil_fancy_get - explain reason for preferring isinstance in scalar fast paths
|
Thanks, revised. Getitem is now ~25% faster due to the faster insertion. |
|
Great! I hope you haven't missed the 0.14 boat :) |
|
To make sure no boats are missed, let's push the merge button. |
ENH: sparse: speed up LIL indexing + assignment via Cython
This PR gains 5...50x speedups in LIL matrix setitem/getitem by writing the fancy indexing code more carefully. I also add fast paths for scalar indexing (~5x faster).
The fancy getitem speed is now not far from CSR/CSC. Fancy setitem is also not far from CSR/CSC (and for changing sparsity pattern, LIL can now be much faster than CSR/CSC).
Benchmarks: https://gist.githubusercontent.com/pv/9102868/raw/a256bfe058b44131f8acba0f41ab90eafacbe127/gistfile1.txt