Implement `select_sorting_periods` in metrics #4302

alejoe91 · 2026-01-07T11:50:15Z

Depends on #4316

Slices a sorting object based on an array ov valid periods. Periods are defined as a structured dtype as:

base_period_dtype = [
    ("start_sample_index", "int64"),
    ("end_sample_index", "int64"),
    ("segment_index", "int64"),
    ("unit_index", "int64"),
]

EDIT:

Refactored computation of spike train metrics, to make sure that periods are consistently taken into account. Added 2 utils functions to compute durations per unit and bin edges per unit, that optionally use the provided periods

src/spikeinterface/core/analyzer_extension_core.py

samuelgarcia · 2026-01-09T10:44:45Z

This is OK for me.
Make a clear documentation somwhere woule help ?

src/spikeinterface/core/sorting_tools.py

src/spikeinterface/core/tests/test_basesorting.py

Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>

src/spikeinterface/core/node_pipeline.py

Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>

alejoe91 · 2026-01-13T12:08:26Z

@chrishalcrow I refactored a few metrics to make sure durations, spike counts, and bins are properly accounted for when slicing with periods. Happy to discuss about this!

…select_sorting_periods_core

alejoe91 · 2026-01-20T16:58:46Z

We don't want to extend these quality metrics to the extension() ? What if someone wants to only get the ISI, CCG or anything else only on the periods? Would it be easy to slice the sorting, and then compute only on the sub sorting? Are the extension robust w.r.t. periods?

I think that extension by extension we could use valid_unit_periods when computed :)

alejoe91 · 2026-01-20T16:58:59Z

@samuelgarcia @chrishalcrow tests added! all good now :)

chrishalcrow · 2026-01-20T17:29:54Z

src/spikeinterface/metrics/utils.py

+    num_segments = len(segment_samples)
+    bin_duration_samples = int(bin_duration_s * sorting.sampling_frequency)
+
+    if periods is not None:


When there are no good periods, then periods will be an empty array? Should we have a general strategy for when this is true - empty units often cause pain/nans...

I added a test for empty periods in num spikes and firing rates. I could add similar tests for all metrics. What do you think?

chrishalcrow · 2026-01-20T18:15:41Z

src/spikeinterface/metrics/spiketrain/metrics.py

    """
-
    sorting = sorting_analyzer.sorting
+    sorting = sorting.select_periods(periods)


When I choose periods so that only one unit is good, and get the num spikes, get

import spikeinterface.full as si import numpy as np from spikeinterface.core.base import unit_period_dtype rec, sort = si.generate_ground_truth_recording(seed=1205) sa = si.create_sorting_analyzer(sort, rec) periods = np.array((0,1000,4000,0), dtype=unit_period_dtype) quality_metrics = si.compute_quality_metrics(sa, periods=periods, metric_names = ['num_spikes']) print(quality_metrics['num_spikes'].values)

>>> [162 173 135 154 158 144 151 141 153 162]

But this sorting only has spikes from unit index 0, right? Is count_num_spikes_per_unit below using an old cache or something?

good point! Thanks for testing! Let me check ;)

Fixed now! I was missing the support_periods class attributes to the spike train metrics

Also added similar tests there :)

chrishalcrow · 2026-01-21T11:22:53Z

src/spikeinterface/metrics/quality/misc_metrics.py

+    cumulative_segment_samples = np.cumsum([0] + segment_samples[:-1])
    for unit_id in unit_ids:
-        firing_histograms = []
+        if num_spikes[unit_id] == 0:


Hello - this num_spikes seems to be the number of spikes of the original sorting before unit periods are selected, which causes problems down the line.

No it's after, see https://github.com/SpikeInterface/spikeinterface/pull/4302/changes#diff-6dd31db7fc9ed3745b7eb4765b7c107663b51745c246860c36b2517019bc75c3R644

samuelgarcia · 2026-01-21T13:37:55Z

src/spikeinterface/core/analyzer_extension_core.py

        if cls.needs_job_kwargs:
            args += (job_kwargs,)
+        if cls.supports_periods:
+            args += (periods,)


Suggested change

args += (periods,)

args += (periods,)

else:

raise ValueError("This metric do not support periods")

samuelgarcia · 2026-01-21T13:40:20Z

src/spikeinterface/core/analyzer_extension_core.py

        if metric_names is None:
            metric_names = self.params["metric_names"]

+        periods = self.params.get("periods", None)


I guess this is to be backward compatible.
I think it would be better to make the caompatibility in the _handle_backward_compatibility_on_load no ?

samuelgarcia · 2026-01-21T13:59:59Z

src/spikeinterface/metrics/quality/tests/test_metrics_functions.py

+
+    # can't use _misc_metric_name_to_func as some functions compute several qms
+    # e.g. isi_violation and synchrony
+    quality_metrics = [


this could be infered by the classe no ?

samuelgarcia · 2026-01-21T14:09:36Z

src/spikeinterface/metrics/quality/misc_metrics.py

+            spike_locations_in_bin = spike_locations_array[i0:i1][direction]
+
+            unit_index = sorting_analyzer.sorting.id_to_index(unit_id)
+            mask = spikes_in_bin["unit_index"] == unit_index


is this fast ?

samuelgarcia · 2026-01-21T14:11:24Z

src/spikeinterface/metrics/quality/utils.py

-import numpy as np
-
-
-def create_ground_truth_pc_distributions(center_locations, total_points):


not needed anymore ?

samuelgarcia · 2026-01-21T14:14:35Z

src/spikeinterface/metrics/utils.py

+        sorting = sorting_analyzer.sorting
+        for unit_id in sorting.unit_ids:
+            unit_index = sorting.id_to_index(unit_id)
+            periods_unit = periods[periods["unit_index"] == unit_index]


I have the intuition that lloping only once over periods and suming directly in a prepared vector will be way faster than this repetition of masking in the loop.
No ?

samuelgarcia · 2026-01-21T14:15:38Z

src/spikeinterface/metrics/utils.py

+                num_samples_in_period += period["end_sample_index"] - period["start_sample_index"]
+            total_samples[unit_id] = num_samples_in_period
+    else:
+        total_samples = {unit_id: sorting_analyzer.get_total_samples() for unit_id in sorting_analyzer.unit_ids}


Suggested change

total_samples = {unit_id: sorting_analyzer.get_total_samples() for unit_id in sorting_analyzer.unit_ids}

total = sorting_analyzer.get_total_samples()

total_samples = {unit_id: total for unit_id in sorting_analyzer.unit_ids}

samuelgarcia · 2026-01-21T14:16:54Z

src/spikeinterface/metrics/utils.py

+        samples_per_period = sorting_analyzer.get_num_samples(segment_index) // num_periods
+        if bin_size_s is not None:
+            bin_size_samples = int(bin_size_s * sorting_analyzer.sampling_frequency)
+            print(samples_per_period / bin_size_samples)


samuelgarcia · 2026-01-21T14:18:03Z

src/spikeinterface/metrics/utils.py

+    return total_durations
+
+
+def compute_periods(sorting_analyzer, num_periods, bin_size_s=None):


I think we should find a better for this.
Like create_regular_periods or something.
The compute is suggesting an algo method.
No ?

samuelgarcia · 2026-01-21T14:18:18Z

src/spikeinterface/metrics/utils.py

+    return np.concatenate(all_periods)
+
+
+def create_ground_truth_pc_distributions(center_locations, total_points):


ok
moved here

samuelgarcia · 2026-01-21T14:18:52Z

Globally OK for me
I made a few comment and some of then should be discussed

alejoe91 added 2 commits December 29, 2025 15:43

Test IBL extractors tests failing for PI update

3dc5729

Merge branch 'main' of github.com:SpikeInterface/spikeinterface

d1a0532

alejoe91 added the core Changes to core module label Jan 7, 2026

alejoe91 mentioned this pull request Jan 7, 2026

Valid unit periods extension #4299

Open

1 task

alejoe91 added 3 commits January 7, 2026 16:26

wip

7279b67

Fix test for base sorting and propagate to basevector extension

1962f21

Fix tests in quailty metrics

528c82b

alejoe91 marked this pull request as ready for review January 8, 2026 07:36

alejoe91 requested a review from chrishalcrow January 8, 2026 07:36

Fix retrieval of spikevector features

775dda7

samuelgarcia reviewed Jan 9, 2026

View reviewed changes

src/spikeinterface/core/analyzer_extension_core.py Outdated Show resolved Hide resolved

chrishalcrow reviewed Jan 12, 2026

View reviewed changes

src/spikeinterface/core/sorting_tools.py Outdated Show resolved Hide resolved

chrishalcrow reviewed Jan 12, 2026

View reviewed changes

src/spikeinterface/core/sorting_tools.py Outdated Show resolved Hide resolved

chrishalcrow reviewed Jan 12, 2026

View reviewed changes

src/spikeinterface/core/tests/test_basesorting.py Outdated Show resolved Hide resolved

Update src/spikeinterface/core/sorting_tools.py

bb46f27

Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>

alejoe91 commented Jan 13, 2026

View reviewed changes

src/spikeinterface/core/node_pipeline.py Outdated Show resolved Hide resolved

Apply suggestion from @chrishalcrow

121a0b1

Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>

alejoe91 mentioned this pull request Jan 13, 2026

Move structured dtypes to base #4314

Merged

refactor presence ratio and drift metrics to use periods properly

cbf3213

alejoe91 added 6 commits January 13, 2026 13:16

Fix rp_violations

4409aa5

implement firing range and fix drift

71f8668

fix naming issue

1ea0d68

remove solved todos

a86c2d3

Merge branch 'main' of github.com:SpikeInterface/spikeinterface into …

d8e1f90

…select_sorting_periods_core

Implement select_segment_periods in core

3f93f97

alejoe91 changed the title ~~Implement select_sorting_periods~~ Implement select_sorting_periods in metrics Jan 13, 2026

alejoe91 added 2 commits January 13, 2026 16:43

remove utils

cd85456

rebase on SpikeInterface#4316

7a42fe3

alejoe91 added 8 commits January 16, 2026 14:52

Solve conflicts

873a687

Fix error in merging

51e906a

fix conflicts

ab5a771

Add supports_periods in BaseMetric/Extension

2209514

wip: test metrics with periods

b23c431

almost there?

6fb26a4

Fix periods arg in MetricExtensions

0fe7f3e

Make bin edges unique

f087e08

chrishalcrow reviewed Jan 20, 2026

View reviewed changes

alejoe91 added 2 commits January 21, 2026 10:05

Add support_periods to spike train metrics and tests

173e747

Force NaN/-1 values for float/int metrics if num_spikes is 0

066c378

chrishalcrow reviewed Jan 21, 2026

View reviewed changes

alejoe91 added 4 commits January 21, 2026 12:27

Fix test_empty_units: -1 is a valid value for ints

65e1848

Fix firing range if unit samples < bin samples

f1c4682

fix noise_cutoff if empty units

3291638

Move warnings at the end of the loop for firing range and drift

b5bf3c3

samuelgarcia reviewed Jan 21, 2026

View reviewed changes

		import numpy as np


		def create_ground_truth_pc_distributions(center_locations, total_points):

	total_samples = {unit_id: sorting_analyzer.get_total_samples() for unit_id in sorting_analyzer.unit_ids}
	total = sorting_analyzer.get_total_samples()
	total_samples = {unit_id: total for unit_id in sorting_analyzer.unit_ids}

		return total_durations


		def compute_periods(sorting_analyzer, num_periods, bin_size_s=None):

		return np.concatenate(all_periods)


		def create_ground_truth_pc_distributions(center_locations, total_points):

Implement select_sorting_periods in metrics #4302

Are you sure you want to change the base?

Implement select_sorting_periods in metrics #4302

Conversation

alejoe91 commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

samuelgarcia commented Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alejoe91 commented Jan 13, 2026

Uh oh!

alejoe91 commented Jan 20, 2026

Uh oh!

alejoe91 commented Jan 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samuelgarcia commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Implement `select_sorting_periods` in metrics #4302

Implement `select_sorting_periods` in metrics #4302

alejoe91 commented Jan 7, 2026 •

edited

Loading