Skip to content

Conversation

@tayheau
Copy link
Contributor

@tayheau tayheau commented Dec 23, 2025

Following this todo

@tayheau tayheau marked this pull request as draft December 23, 2025 12:11
@tayheau tayheau marked this pull request as ready for review December 23, 2025 13:24
@alejoe91 alejoe91 added core Changes to core module testing Related to test routines labels Dec 23, 2025
@alejoe91 alejoe91 added this to the 0.104.0 milestone Dec 23, 2025
@alejoe91
Copy link
Member

@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! :)

spikes.append(spikes_in_seg)

spikes = np.concatenate(spikes)
spikes = spikes[np.lexsort((spikes["unit_index"], spikes["sample_index"], spikes["segment_index"]))]
Copy link
Member

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"]))]
Copy link
Member

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

Comment on lines +298 to +299
spikes = spikes[np.lexsort((spikes["unit_index"], spikes["sample_index"], spikes["segment_index"]))]
self._cached_spike_vector = spikes
Copy link
Member

Choose a reason for hiding this comment

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

and in zarr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes to core module testing Related to test routines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants