Skip to content

Reorder spike vectors with a flexible-dtype numba kernel - #4784

Open
grahamfindlay wants to merge 2 commits into
SpikeInterface:mainfrom
grahamfindlay:perf/reorder-record-kernel
Open

grahamfindlay wants to merge 2 commits into
SpikeInterface:mainfrom
grahamfindlay:perf/reorder-record-kernel

Conversation

@grahamfindlay

Copy link
Copy Markdown
Contributor

Follows @samuelgarcia 's comment on #4695 and #4781.

The numba kernel from #4695 moved rows through a flat (N, num_fields) int64 view and addressed unit_index and segment_index as columns 1 and 2, assuming minimum_spike_dtype.

Samuel suggest that we might want to change the spike vector dtype in the future, so this is a more flexible variant that can take narrow index dtypes (e.g. int8).

Also updated the numpy fallback to work for narrow index fields.

Measured on ~400M spikes, 342 units:

  • 11.6 -> 12.0 s (3x int64 fields)
  • 13.1 -> 13.0 s (4x int64 fields)
  • 9.1 s (1 each of int64/int32/int8 fields)

@alejoe91 alejoe91 added core Changes to core module performance Performance issues/improvements labels Sep 16, 2026
@grahamfindlay

Copy link
Copy Markdown
Contributor Author

The 2 failing tests don't appear to have anything to do with this PR. They're tolerance tests failing on test_decimate_antialias_by_chunks which funnily enough passed in its own PR (#4621) with the same scipy and numpy versions, so they must be nondeterministic. Maybe create_sinusoidal_traces just needs a seed. It looks like they're still tiny tiny single-sample discrepancies. It must be a rounding thing on Apple Silicon.

@grahamfindlay

grahamfindlay commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Yes, I know it was kinda naughty of me to push a fix for that to this PR... forgive me.

EDIT: No longer applicable, see below.

grahamfindlay and others added 2 commits September 17, 2026 11:38
The counting-sort kernel from SpikeInterface#4695 moved rows through a flat (N,
num_fields)
int64 view and addressed unit_index and segment_index as columns 1 and
2, essentially assuming minimum_spike_dtype.

Samuel suggest that we might want to change the spike vector dtype in
the future, so this is a more flexible variant that can take narrow
index dtypes (e.g. int8).

Also updated the numpy fallback to work for narrow index fields.

Measured on ~400M spikes, 342 units:
- 11.6 -> 12.0 s (3x int64 fields)
- 13.1 -> 13.0 s (4x int64 fields)
- 9.1 s (1 each of int64/int32/int8 fields)
@grahamfindlay
grahamfindlay force-pushed the perf/reorder-record-kernel branch from 62a512f to 22cd36f Compare September 17, 2026 16:42
@grahamfindlay

Copy link
Copy Markdown
Contributor Author

Removed the fix for the unrelated failing tests since another one was already just merged in #4792

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