Dark Mode

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Implement ChunkExecutor#4216

Open
alejoe91 wants to merge 33 commits intoSpikeInterface:mainfrom
alejoe91:base-chunk-executor
Open

Implement ChunkExecutor#4216
alejoe91 wants to merge 33 commits intoSpikeInterface:mainfrom
alejoe91:base-chunk-executor

Conversation

Copy link
Member

alejoe91 commented Nov 14, 2025 *
edited
Loading

This PR makes a ChunkExecutor which is agnostic to the recording and applies to any chunkable BaseExtractor (which defines get_sample_size(), get_num_samples(), get_num_segments()

This is because we are working on a new "spikeinterface" for calcium imaging, and this refactor makes it suuuuper straightforward to use the same parallelization machinery for Imaging objects (which inherit from the BaseExtractor)

Update 29/11/25

Following up on trying to make SI tools as general as possible for its calcium imaging "clone" (photon-mosaic) I made some more dramatic refactoring:

  • implemented a ChunkableMixin abstract mixin that implements all method needed by the ChunkExecutor
    • several BaseRecording specific methods (e.g., write_binary/write_memory and more) are now in the chunkable_tools and applicable to any ChunkableMixin object
    • the ChunkableMixin has a general get_data function, so it can be used by many derived classes (e.g., get_traces for recordings, get_videos/series for imaging)
    • another useful function to centralize things is the get_shape(segment_index), which returns the buffer shape by segment
  • refactored "segments": I that we didn't have any proper structure for segments: recording objects had _recording_segments and sorting objects _sorting_segments. Each class then implemented its own add_* function. Since segments are a BaseExtractor concept, I centralized the definition (self.segments: List[BaseSegment] = []), added a self.segments property, and a centralized add_segment function.

@samuelgarcia @chrishalcrow @yger @h-mayorquin @JoeZiminski @zm711

Let me know what you think!!!

alessandrofelder and JoeZiminski reacted with eyes emoji
alejoe91 added core Changes to core module concurrency Related to parallel processing labels Nov 14, 2025
alejoe91 commented Nov 14, 2025
alejoe91 changed the title Implement BaseChunkExecutor Implement ChunkExecutor Nov 14, 2025
h-mayorquin reviewed Nov 17, 2025


def get_random_recording_slices(
def get_random_slices(
Copy link
Member

samuelgarcia Nov 24, 2025

Choose a reason for hiding this comment

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

This is too neutral no ? Lets find some thing like
get_random_slices_along_time

Copy link
Member Author

alejoe91 Nov 24, 2025

Choose a reason for hiding this comment

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

get_random_time_slices?

Copy link
Member Author

alejoe91 Dec 1, 2025

Choose a reason for hiding this comment

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

now renamed get_random_sample_slices

Copy link
Member

samuelgarcia commented Nov 24, 2025

Lets discuss this in detail. This is lots some semantic changes.
I do not like too much the "extractor" evrywhere.

alejoe91 reacted with thumbs up emoji

Copy link
Member Author

alejoe91 commented Nov 24, 2025

Lets discuss this in detail. This is lots some semantic changes. I do not like too much the "extractor" evrywhere.

What about a Chunkable mixin that implements:

  • get_num_segments()
  • get_sample_size_in_bytes()
  • get_num_samples()
    ?
    Then extractor could become chunkable

alejoe91 added 16 commits November 26, 2025 11:15
alejoe91 added this to the 0.105.0 milestone Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

samuelgarcia samuelgarcia left review comments

h-mayorquin h-mayorquin left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

concurrency Related to parallel processing core Changes to core module

Projects

None yet

Milestone

0.105.0

Development

Successfully merging this pull request may close these issues.

3 participants