Implement select_sorting_periods in metrics#4302
Implement select_sorting_periods in metrics#4302samuelgarcia merged 51 commits intoSpikeInterface:mainfrom
select_sorting_periods in metrics#4302Conversation
|
This is OK for me. |
Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>
Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>
|
@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_periodsselect_sorting_periods in metrics
|
@samuelgarcia @chrishalcrow tests added! all good now :) |
| if metric_names is None: | ||
| metric_names = self.params["metric_names"] | ||
|
|
||
| periods = self.params.get("periods", None) |
There was a problem hiding this comment.
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 ?
src/spikeinterface/metrics/quality/tests/test_metrics_functions.py
Outdated
Show resolved
Hide resolved
src/spikeinterface/metrics/utils.py
Outdated
| 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] |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
this is super fast. Periods will be few..so it's ok!
There was a problem hiding this comment.
plus periods are by unit, so we need to slice it by unit anyways!
|
Globally OK for me |
chrishalcrow
left a comment
There was a problem hiding this comment.
Tested for speed and result regressions - all ok! Happy with the PR :)
Thanks Alessio!
Depends on #4316
Slices a sorting object based on an array ov valid periods. Periods are defined as a structured dtype as:
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