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

Enhancing curation : get_potential_auto_merge() #2753

Merged
merged 11 commits into from
Apr 30, 2024

Conversation

yger
Copy link
Collaborator

@yger yger commented Apr 24, 2024

This PR improve get_potential_auto_merge() with:

  • several metrics for templates distances
  • handling sparsity of templates
  • use computed units locations from analyzer

@DradeAW : can you check this, if you use this in Lussac. Normally the behavior is the same as before.

@zm711 zm711 added the curation Related to curation module label Apr 25, 2024
@samuelgarcia samuelgarcia changed the title Enhancing syntax for lussac merging Enhancing curation : get_potential_auto_merge() Apr 26, 2024
@samuelgarcia
Copy link
Member

@alejoe91 this is OK for me. Pierre will need this for circus.

@DradeAW
Copy link
Contributor

DradeAW commented Apr 26, 2024

I pulled the changed and ran the Lussac unit tests (which check that units that should be merged are indeed merged)

I get this error:

tests/modules/test_merge_units.py:51: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
src/lussac/modules/merge_units.py:56: in run
    potential_merges, extra_outputs = scur.get_potential_auto_merge(self.analyzer, extra_outputs=True, **params['auto_merge_params'])
../../dev/spikeinterface/spikeinterface/src/spikeinterface/curation/auto_merge.py:207: in get_potential_auto_merge
    templates = templates_ext.get_data(outputs="Templates")
../../dev/spikeinterface/spikeinterface/src/spikeinterface/core/sortinganalyzer.py:1704: in get_data
    return self._get_data(*args, **kwargs)
../../dev/spikeinterface/spikeinterface/src/spikeinterface/core/analyzer_extension_core.py:430: in _get_data
    probe=self.sorting_analyzer.get_probe(),
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = SortingAnalyzer: 16 channels - 15 units - 1 segments - memory - has recording
Loaded 2 extensions: random_spikes, templates

    def get_probe(self):
        probegroup = self.get_probegroup()
>       assert len(probegroup.probes) == 1, "There are several probes. Use `get_probegroup()`"
E       AssertionError: There are several probes. Use `get_probegroup()`

../../dev/spikeinterface/spikeinterface/src/spikeinterface/core/sortinganalyzer.py:776: AssertionError

@yger yger mentioned this pull request Apr 26, 2024
@yger
Copy link
Collaborator Author

yger commented Apr 26, 2024

Aie. The bug seems to be more in the sorting_analyzer object. Or I should catch the fact that maybe the get_data() for the "templates" extension is not properly able to return Templates when more than one probes are selected. What do you think @samuelgarcia ? Otherwise, if more than one probe is detected, then we get Templates with the 'average' method, as it used to be done before. The following function will work properly since it can handled Templates or numpy arrays as input

…auto_merge() when analyzer has several probes.
@samuelgarcia
Copy link
Member

@DradeAW I made a fix for multiple probe.
Can check this again please ?

@DradeAW
Copy link
Contributor

DradeAW commented Apr 30, 2024

All tests have passed on my end! :)

@samuelgarcia samuelgarcia merged commit c0bfb49 into SpikeInterface:main Apr 30, 2024
11 checks passed
@yger yger deleted the curation_improvements branch May 6, 2024 15:26
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.

4 participants