Skip to content

Fix main_channel_id donor selection when merged units disagree - #4773

Merged
alejoe91 merged 3 commits into
SpikeInterface:mainfrom
JESUSROYETH:fix/main-channel-id-merge-order
Sep 10, 2026
Merged

alejoe91 merged 3 commits into
SpikeInterface:mainfrom
JESUSROYETH:fix/main-channel-id-merge-order

Conversation

@JESUSROYETH

Copy link
Copy Markdown
Contributor

merge_units() gives the merged unit main_channel_id from whichever unit is listed first in the merge group on any disagreement, not from the one with the most spikes like the comment says: np.argmax(num_spikes_per_unit.values()) runs on a dict_values view NumPy can't iterate (no __getitem__), so it always returns 0.

Minimal proof: 3 units with spike counts 8/5/7, merging ["b", "a"] returns chB (fewer spikes) instead of chA on main. main_channel_id is set by default for every SortingAnalyzer since #4624.

Fix: look the count up by unit id instead of trusting argmax/position. Doesn't touch the neighbouring note about recomputing from templates for analyzers — only makes the existing spike-count fallback do what its own comment says.

Two regression tests (test_sorting_tools.py, test_sortinganalyzer.py, covering both apply_merges_to_sorting and SortingAnalyzer.merge_units()) fail on main, pass patched; 48 tests total, black clean.

@alejoe91 alejoe91 added the core Changes to core module label Sep 10, 2026
@alejoe91 alejoe91 added this to the 0.105.0 milestone Sep 10, 2026
@chrishalcrow chrishalcrow self-assigned this Sep 10, 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.

This is great, thanks!

@alejoe91
alejoe91 merged commit bf0f01b into SpikeInterface:main Sep 10, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes to core module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants