-
Notifications
You must be signed in to change notification settings - Fork 241
using spike_vector on core/check_sortings_equal test
#4282
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
using spike_vector on core/check_sortings_equal test
#4282
Conversation
for more information, see https://pre-commit.ci
|
@tayheau I think we actually needed the unit_index lex sort after all! The reason is that before we were testing spiketrain-by-spiketrain, so the order didn't matter if we had synchronous spikes. Now with the spike vector representation we need to ensure unit_index is sorted too! Sorry about that! :) |
for more information, see https://pre-commit.ci
…spike_vector_test_check_sorting_equal
…yheau/spikeinterface into spike_vector_test_check_sorting_equal
| spikes.append(spikes_in_seg) | ||
|
|
||
| spikes = np.concatenate(spikes) | ||
| spikes = spikes[np.lexsort((spikes["unit_index"], spikes["sample_index"], spikes["segment_index"]))] |
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.
@samuelgarcia I think it's better to lexsort the whole array at the end to make sure that units are also sorted within samples
|
|
||
| spikes = np.concatenate(spikes) | ||
| spikes = spikes[np.lexsort((spikes["sample_index"], spikes["segment_index"]))] | ||
| spikes = spikes[np.lexsort((spikes["unit_index"], spikes["sample_index"], spikes["segment_index"]))] |
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.
we were missing the unit index lexsort in generate
| spikes = spikes[np.lexsort((spikes["unit_index"], spikes["sample_index"], spikes["segment_index"]))] | ||
| self._cached_spike_vector = spikes |
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.
and in zarr
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.
Doing this in the init is an enormous cost in term of perfs.
I am not sure this is a good idea to lexsort at each reading.
lets discuss
src/spikeinterface/core/testing.py
Outdated
| mask &= spikes["sample_index"] >= start_frame | ||
| if end_frame is not None: | ||
| mask &= spikes["sample_index"] <= end_frame |
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.
searchsorted should be faster no ?
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.
yep indeed
|
thanks @tayheau Merging! |
Following this todo