Skip to content

Fix: grid convolution select_peaks - #4863

Merged
alejoe91 merged 2 commits into
SpikeInterface:mainfrom
alejoe91:fix-select-peaks-with-channel-index
Oct 14, 2026
Merged

alejoe91 merged 2 commits into
SpikeInterface:mainfrom
alejoe91:fix-select-peaks-with-channel-index

Conversation

@alejoe91

@alejoe91 alejoe91 commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Another bug in postprocessing (spike locations) which we didn't catch with tests.

The issue is that in #4809 we introduced the possibility to compute locations from spikes (no channel_index), so we added a way to pass channel_index_array externally. When computing spike locations with grid convolution, we have to slice the channel_index_array according to the peak selection. This was not caught by tests since the postprocessing dataset has 163 spikes, and default peak selection is 5000!

Extended tests to select 100 peaks for grid convolution.

@alejoe91 alejoe91 added this to the 0.105.2 milestone Oct 14, 2026
@alejoe91
alejoe91 requested a review from chrishalcrow October 14, 2026 09:27
@alejoe91 alejoe91 added bug Something isn't working sortingcomponents Related to sortingcomponents module postprocessing Related to postprocessing module labels Oct 14, 2026

@chrishalcrow chrishalcrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@alejoe91
alejoe91 merged commit dd4be69 into SpikeInterface:main Oct 14, 2026
12 checks passed
@alejoe91
alejoe91 deleted the fix-select-peaks-with-channel-index branch October 14, 2026 10:19
alejoe91 added a commit to alejoe91/spikeinterface that referenced this pull request Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working postprocessing Related to postprocessing module sortingcomponents Related to sortingcomponents module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants