Repository navigation
Speed up soft merges with vectorized spike remapping - #4848
Open
JESUSROYETH wants to merge 1 commit into
Open
JESUSROYETH wants to merge 1 commit into
JESUSROYETH wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
apply_merges_to_sorting()currently builds absolute spike indices for every unit and segment, then loops over them to rewriteunit_index. It does this even whencensor_ms=None, but that path only needs a direct old-to-new unit mapping.This PR replaces the per-unit index building with a compact lookup table applied to the spike vector in one NumPy operation. The old index-building path stays for censored merges, where it is still needed.
The public signature and
return_extra=Truebehavior are unchanged. Thekeep_maskdocstring is corrected too: it is only returned whenreturn_extra=True, and it is always a boolean mask (allTruewhencensor_msisNone). It was never actuallyNoneon that path, so the old docstring was simply wrong.I measured the complete public
SortingAnalyzer.merge_units()call, including propagation of eight loaded extensions, on a one-hour generated analyzer with 300 units, 384 channels and 5,397,781 spikes. Five paired runs after one warm-up ran on a fresh GCPc2-standard-8:Every pair improved by 12.97-15.91%, and every baseline/candidate run had the same SHA-256 across the merged spike vector and all deterministic extension outputs. The public 10-second MEARec fixture gave identical outputs too, and a 200-case randomized sweep covered multi-segment, multi-group and censored merges.
A new focused test also covers the
return_extra=True, censor_ms=Nonecombination, so the unchangedkeep_maskcontract for that branch now has a test behind it, not just the docstring. The full sorting-tools test file, plus the memory,binary_folderandzarranalyzer cases, all pass (34 tests). Black and the repository style checks are clean.Related to #4310.