Skip to content

Fix and speed up to_reordered_spike_vector - #4695

Open
grahamfindlay wants to merge 5 commits into
SpikeInterface:mainfrom
grahamfindlay:perf/to-reordered-spike-vector
Open

Fix and speed up to_reordered_spike_vector#4695
grahamfindlay wants to merge 5 commits into
SpikeInterface:mainfrom
grahamfindlay:perf/to-reordered-spike-vector

Conversation

@grahamfindlay

@grahamfindlay grahamfindlay commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Follows #4606 and #4683. Implements part of #4502. @alejoe91 @samuelgarcia

  1. Fixes the always true elif ("sample_index", "unit_index", "segment_index"): in to_reordered_spike_vector.
  2. Restricts to_reordered_spike_vector to non-vanilla orderings, and fixes some inaccuracies in the docstring.
  3. Optimizes to_reordered_spike_vector. There is a numba path with a counting sort that drops the time on an ~400M spike / 342 unit / 1 segment sorting from 137s to 12s, with no additional memory usage. There is a numpy fallback path using a radix sort that drops the time from 137s to 85s, with a slightly increased memory cost (34 bytes/spike instead of 32 bytes/spike). The outputs (ordered_spikes, slices, order) are identical to the old implementation. I also tested on synthetic data of various sizes (1M, 10M, 40M spikes) to make sure both paths are strict improvements at all input sizes.
  4. Fixes a bug where the reorder cache was set using a string, then probed by various callers using a tuple (or list), so the cache was being silently missed basically all the time. I defined some constants for the keys and used them everywhere, and made the validation stricter, to minimize the likelihood that it happens again.

@alejoe91 alejoe91 added core Changes to core module performance Performance issues/improvements labels Jul 17, 2026
@alejoe91 alejoe91 added this to the 0.105.0 milestone Jul 17, 2026
…erface#4683)

Also fixes the docstring, which was wrong in two places: it described the
pre-SpikeInterface#4606 vanilla order, and documented a default that was not the actual
default.
Adds reorder_spike_vector_by_unit_and_segment() to sorting_tools.
With numba, this is a counting sort. Without, it is a stable argsort on
a bucket narrowed to the smallest dtype that fits, which lets numpy
radix sort it.

On ~400M spikes, 342 units, 1 segment, to_reordered_spike_vector goes
from 137s to:

* ~12s with numba (~11x), at unchanged peak memory usage
* ~85s with numpy (~1.6x), for +2 bytes/spike (the narrowed bucket)

ordered_spikes, slices and order are identical to the previous
implementation. Also tested on synthetic recordings of various sizes.
to_reordered_spike_vector was caching under str(lexsort),
but the three readers of _cached_lexsorted_spike_vector --
get_unit_spike_train, count_num_spikes_per_unit and
precompute_spike_trains -- probed it with the tuple, so
those lookups never matched, and the cache was silently
missing every time.

The bug was possible because the same lexsort literal had to be retyped
at eight sites, and they drifted apart.

Using constants LEXSORT_UNIT_COMPACT / LEXSORT_SEGMENT_COMPACT
is not a perfect solution (it would probably be better not to
reach into the private dictionary in so many places), but it
it should at least minimize the likelihood that the same mistake
gets made again.
backing.

reorder_spike_vector_by_unit_and_segment assumed a three-field int64
in-memory array. However, spike vectors can have extra fields (e.g.
"channel_index", also int64), including arbitrary user-defined fields
(e.g. "amplitude", maybe float64). They can also be lazy (e.g.
ZarrSpikeVector).

Updated the numba kernel to take extra int64 field(s).
Anything spike vector with a non-int64 field now gets routed through the
numpy fallback path (which is dtype-agnsotic).
Lazy vectors are normalised with np.asarray (which np.lexsort
was already doing implicitly).
@grahamfindlay
grahamfindlay force-pushed the perf/to-reordered-spike-vector branch from 7dea432 to f1ff67c Compare August 7, 2026 23:28
@grahamfindlay

Copy link
Copy Markdown
Contributor Author

I was refreshing this to keep it up-to-date with main, and learned from two of the intervening PRs that (1) spike vectors can be lazy and (2) can have extra fields like channel_index. Then while trying to figure out what other fields can exist, I learned that they don't even have to all be int64. So I updated this PR to handle all those cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes to core module performance Performance issues/improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants