-
Notifications
You must be signed in to change notification settings - Fork 190
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
Sparse waveforms for Spyking Circus 2 #1943
Conversation
… into factoring_omp
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
mode = "folder" | ||
|
||
sorting_folder = tmp_folder / "sorting" | ||
sorting = NumpySorting.from_times_labels(spikes["sample_index"], spikes["unit_index"], fs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is faster and should be the default when we already have a spike vector.
Note the you need to given also unit_ids
sorting = NumpySorting.from_times_labels(spikes["sample_index"], spikes["unit_index"], fs) | |
sorting = NumpySorting(spikes, fs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yger this is the only change that is required! Can you add the unit_ids
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. However, I have a question for the new waveform speedup, with single or multi buffers. @samuelgarcia @alejoe91 shouldn't this functionnality be exposed at the extract_waveforms() level?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is the plan but not now.
And WaveformEtractor will be refactor a lot anyway not even sure this will be done before the big refactoring.
ok for me. |
for more information, see https://pre-commit.ci
@samuelgarcia I've checked the behaviour, and this is working fine, so I think you can proceed with merging this PR |
PR to provide the support for sparse waveforms in circus 2, and factorized matching engine. The internal machinery now uses the sparsification mecanisms provided by SI, and thus we should be able to benchmark properly that among sorters