Skip to content

Add threshold_metrics curation#4365

Open
alejoe91 wants to merge 14 commits intoSpikeInterface:mainfrom
alejoe91:qualitymetrics_curation
Open

Add threshold_metrics curation#4365
alejoe91 wants to merge 14 commits intoSpikeInterface:mainfrom
alejoe91:qualitymetrics_curation

Conversation

@alejoe91
Copy link
Member

@alejoe91 alejoe91 commented Feb 3, 2026

In the spirit of model_based_label_units and bombcell_label_units, this PR adds qualitymetrics_label_units

@alejoe91 alejoe91 requested a review from chrishalcrow February 3, 2026 09:53
@alejoe91 alejoe91 added the curation Related to curation module label Feb 3, 2026
@samuelgarcia samuelgarcia added this to the 0.104.0 milestone Feb 3, 2026
@Julie-Fabre
Copy link

My kind of high level idea was people can add or remove threshold in the bombcell_label_units and that is then equivalent to this (if people set thresholds to none for metrics other than fr and snr) but people also get the output plots spit out and (eventually) the integration in the GUI. We could call bombcell just " quality metrics" and then have everything alltogether - does it make sense to duplicate things or am I missing how this is different?

@alejoe91 alejoe91 changed the title Add qualitymetrics curation Add threshold_metrics curation Feb 4, 2026
@alejoe91
Copy link
Member Author

alejoe91 commented Feb 4, 2026

My kind of high level idea was people can add or remove threshold in the bombcell_label_units and that is then equivalent to this (if people set thresholds to none for metrics other than fr and snr) but people also get the output plots spit out and (eventually) the integration in the GUI. We could call bombcell just " quality metrics" and then have everything alltogether - does it make sense to duplicate things or am I missing how this is different?

Hi @Julie-Fabre

I think that bombcell is more sophisticated than this! You can define different rules for different thresholds (e.g., noise/non-soma etc). Maybe what we could do is the following:

  • extend this function to be able to externally set labels
  • use this function in cascade within bombcell internally, to make the various labeling

I also think it makes sense to keep bombcell since there is a lot of work and feature "engineering" to get a proper output, so it should receive proper credit!

But I'm open to discussion! @chrishalcrow what do you think?

@m-beau
Copy link
Contributor

m-beau commented Feb 4, 2026

Hey there!

Having a general-purpose function to threshold quality metrics in various ways is great, but I tend to agree with julie - this could very well be implemented as variations from the default parameters of bombcell. After all, the metrics people use should probably be variations from the bombcell values, and I’m sure a lot of the default parameters used here will be from bombcell, as they have been tested and are trusted by people, so it would have the benefit of making this clear for users.

@chrishalcrow
Copy link
Member

I think one of the aims of this was to use it as a educational tool mostly (we also have simple sorter etc!). The idea is: look, you can use some simple thresholds to do a very naive good/bad classification, but you should really do something more sophisticated. We're also laying the groundwork for having a "unitlabel" extension to keep provenance about labeling decision (and making it easy to generate "methods section" which include labeling tools!)

@alejoe91
Copy link
Member Author

alejoe91 commented Feb 4, 2026

@Julie-Fabre @m-beau I also see it more the other way around: this threshold based method can be the building block of bombcell, which does much more than just simple thresholds in terms of multiple steps applied in different orders and rules (e.g., "and" versus "or").

I made a draft implementation of the concept here: https://github.com/SpikeInterface/spikeinterface/pull/4306/changes#diff-1d9dd3572d81d393b756ec67a04c624f15cd208fd6fb67d5c9a1468aff927732R149-R245

@Julie-Fabre
Copy link

Julie-Fabre commented Feb 4, 2026

Thanks for the discussion everyone! Just to share my perspective:

I'm not sure I see the value in duplicating the simplest part of bombcell under a different name, even as an educational tool. Bombcell already does thresholding, plus it generates output plots that help users understand and refine their classifications, and I'm actively working on SI GUI integration. What would threshold_metrics offer that bombcell doesn't already provide, or couldn't provide with minor adjustments to expose simpler options for users who want that, or in a simple example script to get them started? If it's meant as an educational tool, wouldn't you want people using the correct function calls and getting used to looking at the output plots from the start? Curious to hear what you think!

I also feel like progressively decomposing bombcell's logic into simpler components post hoc is always possible, but I don't think it reflects the actual intellectual lineage.

We're also laying the groundwork for having a "unitlabel" extension to keep provenance about labeling decision (and making it easy to generate "methods section" which include labeling tools!)

That’s very cool, nice! I'm very happy this is happening! I worked on some of this but still have not pushed anything, I’ll try to participate with what you’ve already done :)

Edit: let's continue chatting on slack



def threshold_metrics_label_units(
sorting_analyzer_or_metrics: "SortingAnalyzer | pd.DataFrame",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only accept dataframe (sorting_analyzer.get_metrics_extension_data() is easy enough)

Comment on lines 115 to 116
else: # "ignore"
metric_ok |= is_nan
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
else: # "ignore"
metric_ok |= is_nan
elif nan_policy == "pass":
metric_ok |= is_nan

Comment on lines 120 to 123
if operator == "and":
pass_mask &= metric_ok
else:
pass_mask |= metric_ok
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if operator == "and":
pass_mask &= metric_ok
else:
pass_mask |= metric_ok
if nan_policy == "ignore":
if operator == "and":
pass_mask[~is_nan] &= metric_ok[~is_nan]
else:
pass_mask[~is_nan] |= metric_ok[~is_nan]

something like that

alejoe91 and others added 2 commits February 5, 2026 20:21
Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>
Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

curation Related to curation module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants