Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Template similarity lags #2941

Merged
merged 46 commits into from
Jun 26, 2024
Merged

Conversation

yger
Copy link
Collaborator

@yger yger commented May 30, 2024

Add the possibility to specify a max_lag_ms while computing the distance between templates.

@alejoe91 alejoe91 added hackathon-24 Contributions during the SpikeInterface Hackathon May 24 postprocessing Related to postprocessing module labels Jun 1, 2024
@alejoe91
Copy link
Member

alejoe91 commented Jun 1, 2024

@yger can we take the chance and add the L1 and L2-norms as metrics for similarity? I think this is done elsewhere already in the sortingcomponents

@yger
Copy link
Collaborator Author

yger commented Jun 4, 2024

@alejoe91 This has been added. The next question here (also for @samuelgarcia) is that do we want to rely on such an extension for the curation metrics ? Currently, all the methods ported from lussac are somehow duplicating code to compute distances between templates, maybe we should (if already computed) stick to the metrics of the extension? If so, I can simply add normalized_l1 as a new metric (to replace the one from Lussac)

@yger
Copy link
Collaborator Author

yger commented Jun 6, 2024

All distances can now be used, and the cosine_similarity has been rename to cosine (but could be discussed). Note that various metrics are also used all over the place in the curation/lussac modules. Up to us to decide if we want to harmonize that and to make them available at this extension level. The problem is that:

  • one might want to compute several distances (and thus handle several extension of the same type)
  • the metric are rather home made and appropriate names need to be found, that will be explicit enough. For example in lussac, l1_normalized need to be explained in the doc.
    Let's discuss if this is an overkill and/or worth developing. These metrics are playing a rather large role for meta merging, so this should not be overlooked.

@yger
Copy link
Collaborator Author

yger commented Jun 9, 2024

This is good to go, and I've just added the metrics used in the curation modules. If this is fine for you, I could now make the curation module depends on these metrics. Note that I think that indeed, the union method is way better than the intersection, currently used in the curation. I've added docs. The only question that remains is that now, all metrics tends to return 0 when templates are similar, except the cosine, where this is 1 when they are similar. Should we consider harmonizing it?

@alejoe91
Copy link
Member

alejoe91 commented Jun 9, 2024

Thanks @yger !! Yes I think that we should.harmonize towards similarity, which is a more common concept. So equal templates would have a similarity of 1, and orthogonal of 0. For l1/l2 norm, since they are distances, it should be simply 1-dist, right?

@yger
Copy link
Collaborator Author

yger commented Jun 9, 2024

Ok, I'll harmonize toward similarity, ensuring that we have always 1 if template are identical. Then I'll integrate this PR into the one from meta merging and we could start playing with that

@samuelgarcia samuelgarcia added this to the 0.101.0 milestone Jun 12, 2024
@alejoe91
Copy link
Member

alejoe91 commented Jun 25, 2024

@yger @samuelgarcia I moved the support/sparsity logic to the compute_similarity_with_templates_array function and extended the template comparison to accept the new arguments (see compute_template_similarity_by_pair)

Had to fix some stuff because the compute_similarity_with_templates_array assumed that the templates arrays were the same, but the are not always the same, so we need to propagate the "other_template" logic everywhere

@samuelgarcia
Copy link
Member

let me read this before merging.

@alejoe91 alejoe91 merged commit 921ec82 into SpikeInterface:main Jun 26, 2024
17 checks passed
@yger yger deleted the template_similarity_lags branch July 5, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hackathon-24 Contributions during the SpikeInterface Hackathon May 24 postprocessing Related to postprocessing module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants