Skip to content

Extend artifact detection: DetectAndRemoveArtifacts preprocessor and signed saturation#4539

Open
alejoe91 wants to merge 45 commits intoSpikeInterface:mainfrom
alejoe91:saturation-preprocessor
Open

Extend artifact detection: DetectAndRemoveArtifacts preprocessor and signed saturation#4539
alejoe91 wants to merge 45 commits intoSpikeInterface:mainfrom
alejoe91:saturation-preprocessor

Conversation

@alejoe91
Copy link
Copy Markdown
Member

@alejoe91 alejoe91 commented Apr 20, 2026

This PR introduces a DetectAndRemoveArtifactsRecording class so that it can be used in the preprocessing pipeline.
Users can pass a recording_to_detect: in this case, saturation/artifacts are detected on that recording (e.g., raw) and applied to the preprocessor parent (preprocessed)

Also extended saturation to optionally provide the "sign" of the saturation events. This can be useful to further inspect saturation issues and track down experimental causes to the saturation.

EDIT (Chris): new preprocessing pipeline docs: https://spikeinterface--4539.org.readthedocs.build/en/4539/modules/preprocessing.html#the-preprocessing-pipeline

@alejoe91 alejoe91 requested a review from samuelgarcia April 20, 2026 15:37
@alejoe91 alejoe91 requested a review from JoeZiminski April 20, 2026 15:37
@alejoe91 alejoe91 added the preprocessing Related to preprocessing module label Apr 20, 2026
@alejoe91 alejoe91 requested a review from chrishalcrow April 20, 2026 15:37
Comment thread src/spikeinterface/preprocessing/detect_artifacts.py Outdated
@chrishalcrow
Copy link
Copy Markdown
Member

Hello, this is great. Part of (my) pipeline philosophy is that you can load a pipeline from a dumped json/pickle:

def _make_pipeline_dict_from_recording_dict(recording_dict):

I don't know how this will work when one argument is a recording object? Might just work?

And I also like the idea that you can specify a pipeline externally without reference to the recording, which this breaks. But more philosophical than practical... I can't really see a way around this.

@alejoe91
Copy link
Copy Markdown
Member Author

alejoe91 commented Apr 21, 2026

Hello, this is great. Part of (my) pipeline philosophy is that you can load a pipeline from a dumped json/pickle:

def _make_pipeline_dict_from_recording_dict(recording_dict):

I don't know how this will work when one argument is a recording object? Might just work?

And I also like the idea that you can specify a pipeline externally without reference to the recording, which this breaks. But more philosophical than practical... I can't really see a way around this.

I have a suggestion. We can have some "magic" string:

  • "pipeline['raw']": tells the function to use the input recording
  • "pipeline['{preprocessing_step}']: grabs recording at a specific step.

What do you think?

Copy link
Copy Markdown
Contributor

@oliche oliche left a comment

Choose a reason for hiding this comment

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

It's good to go, maybe we could state that this apodization is a number of samples ?

Thanks a lot for looking at this !

@alejoe91
Copy link
Copy Markdown
Member Author

It's good to go, maybe we could state that this apodization is a number of samples ?

Thanks a lot for looking at this !

Renamed apodization_samples ;)

Comment thread src/spikeinterface/preprocessing/detect_artifacts.py
self.apodization_samples = apodization_samples

def get_traces(self, start_frame, end_frame, channel_indices):
traces = self.parent_recording_segment.get_traces(start_frame, end_frame, channel_indices)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When it's apodization, we should get traces with margin because the events could be on the edge. In that casem different start/end frames could give different results if we don't take margins into account

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@oliche do you agree?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@samuelgarcia I implemented the margin logic and added a proper test. For zeros and noise, margin is set to 0, so the behavior should be the same

Comment thread src/spikeinterface/preprocessing/tests/test_pipeline.py
Copy link
Copy Markdown
Member

@chrishalcrow chrishalcrow left a comment

Choose a reason for hiding this comment

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

I've only read the preprocessing pipeline stuff - honestly so happy with the solution!
Please don't merge until you've got an approval on the other stuff!

@alejoe91
Copy link
Copy Markdown
Member Author

alejoe91 commented May 6, 2026

Also added automatic loading od saturation levels for SpikeGLX and OpenEphys, follwing SpikeInterface/probeinterface#434

When available, this sets the saturation_threshold_uV property, so saturation detection is automatic!!!

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

Labels

preprocessing Related to preprocessing module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants