Skip to content

Fix closest_channels sparsity estimation when n_units > n_channels#4443

Merged
alejoe91 merged 4 commits into
SpikeInterface:mainfrom
ueeseer:ada/pr1-4126-closest-channels-sparsity
Mar 16, 2026
Merged

Fix closest_channels sparsity estimation when n_units > n_channels#4443
alejoe91 merged 4 commits into
SpikeInterface:mainfrom
ueeseer:ada/pr1-4126-closest-channels-sparsity

Conversation

@ueeseer

@ueeseer ueeseer commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

This fixes #4126 in estimate_sparsity(method="closest_channels").

The previous implementation indexed the channel-distance matrix using unit_ind, which can fail when the number of units exceeds the number of channels. This patch instead selects the nearest channels starting from each unit’s extremum channel, aligning closest_channels with the existing from_radius() logic.

It also adds a targeted regression test covering the n_units > n_channels case.

Local validation

  • ../.venv-spike/bin/pytest -q src/spikeinterface/core/tests/test_sparsity.py -k "estimate_sparsity or compute_sparsity"
  • ../.venv-spike/bin/pytest -q src/spikeinterface/core/tests/test_sparsity.py

@ueeseer

ueeseer commented Mar 15, 2026

Copy link
Copy Markdown
Contributor Author

I rechecked this on the exact current PR head (e22a2c5) before changing anything.

This PR only touches closest_channels sparsity selection in ChannelSparsity.from_closest_channels(). The curation fixture behind test_model_based_classification_predict_labels builds its analyzer with create_sorting_analyzer(..., sparse=True), which uses the default radius-based sparsity path rather than closest_channels.

I also ran test_model_based_classification_predict_labels locally on both origin/main and the exact current PR head (e22a2c5), and it passes in both cases. The curation fixture sparsity masks were also identical in that local comparison.

Given that the only failing cell is macos-latest / Python 3.13, and the observed failure is a one-label prediction drift ([1, 0, 1, 0, 1] -> [1, 0, 1, 0, 0]) rather than a crash or invariant break, this does not currently look causally connected to the #4126 fix. At this point it looks more like an isolated platform-specific prediction drift than a regression in the touched logic.

Comment thread src/spikeinterface/core/sparsity.py Outdated
)
channel_locations = templates_or_sorting_analyzer.get_channel_locations()
distances = np.linalg.norm(channel_locations[:, np.newaxis] - channel_locations[np.newaxis, :], axis=2)
best_chan = get_template_extremum_channel(templates_or_sorting_analyzer, peak_sign="neg", outputs="index")

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.

We should move peak_sign to the args

@alejoe91 alejoe91 added the core Changes to core module label Mar 16, 2026
@alejoe91 alejoe91 added this to the 0.104.0 milestone Mar 16, 2026
@alejoe91

Copy link
Copy Markdown
Member

Thanks for picking this up @ueeseer

Just exposed the peak_sign param as a final touch :)

@alejoe91 alejoe91 merged commit 3255330 into SpikeInterface:main Mar 16, 2026
15 checks passed
@ueeseer ueeseer deleted the ada/pr1-4126-closest-channels-sparsity branch March 16, 2026 15:08
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.

IndexError: index 384 is out of bounds when running estimate_sparsity() with method closest_channels

2 participants