Add threshold_metrics curation#4365
Conversation
|
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:
I also think it makes sense to keep But I'm open to discussion! @chrishalcrow what do you think? |
|
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. |
|
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!) |
|
@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 |
|
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.
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", |
There was a problem hiding this comment.
only accept dataframe (sorting_analyzer.get_metrics_extension_data() is easy enough)
| else: # "ignore" | ||
| metric_ok |= is_nan |
There was a problem hiding this comment.
| else: # "ignore" | |
| metric_ok |= is_nan | |
| elif nan_policy == "pass": | |
| metric_ok |= is_nan |
| if operator == "and": | ||
| pass_mask &= metric_ok | ||
| else: | ||
| pass_mask |= metric_ok |
There was a problem hiding this comment.
| 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
Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>
Co-authored-by: Chris Halcrow <57948917+chrishalcrow@users.noreply.github.com>
In the spirit of
model_based_label_unitsandbombcell_label_units, this PR addsqualitymetrics_label_units