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

Implement syncrhrony metrics #1951

Merged
merged 11 commits into from
Sep 7, 2023
Merged

Conversation

alejoe91
Copy link
Member

@alejoe91 alejoe91 commented Sep 1, 2023

Replaces #1205

@musall @Moritz-Alexander-Kern

Implements synchrony metrics low-level, without depending on elephant.

Default values for synchrony sizes are 2, 4, and 8. Sizes <= 1 throuw an error, as every spike will have a complexity of at least 1.

TODO

  • Add function to add syncrhonous events to existing sorting (might be useful)
  • Add tests: assess if syncrony metrics increase with added levels of synchrony

@alejoe91 alejoe91 added the qualitymetrics Related to qualitymetrics module label Sep 1, 2023
Copy link
Collaborator

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

couple typo suggestions, and a wording clarification.

doc/modules/qualitymetrics/synchrony.rst Outdated Show resolved Hide resolved
doc/modules/qualitymetrics/synchrony.rst Outdated Show resolved Hide resolved
@alejoe91 alejoe91 marked this pull request as ready for review September 1, 2023 14:26
@alejoe91
Copy link
Member Author

alejoe91 commented Sep 1, 2023

@samuelgarcia thanks for the tip! The implementation with np.unique is indeed faster (only 4X)

image

@alejoe91
Copy link
Member Author

alejoe91 commented Sep 6, 2023

@samuelgarcia is this ok to merge?

@samuelgarcia
Copy link
Member

@musall @Moritz-Alexander-Kern : would you have time to look to this to valide that this new code correcpond to the method ?

@alejoe91
Copy link
Member Author

alejoe91 commented Sep 6, 2023

@musall @Moritz-Alexander-Kern : would you have time to look to this to valide that this new code correcpond to the method ?

I can point to a gist that shows equivalence with the elephant implementation.

@alejoe91
Copy link
Member Author

alejoe91 commented Sep 6, 2023

Here it is:

https://gist.github.com/alejoe91/1a10a10420901599db1cc77789a2f714

@samuelgarcia samuelgarcia merged commit c9a5136 into SpikeInterface:main Sep 7, 2023
1 check passed
@musall
Copy link

musall commented Sep 7, 2023

The code looks good to me.
Thank you for pushing the new implementation!

@Moritz-Alexander-Kern
Copy link

I agree, thanks for the implementation.

@alejoe91 alejoe91 deleted the synchronicity branch October 17, 2023 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qualitymetrics Related to qualitymetrics module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants