SpikeInterface / spikeinterface

A Python-based module for creating flexible and robust spike sorting pipelines.
https://spikeinterface.readthedocs.io
MIT License
532 stars 187 forks source link

Peaks (node_pipeline) detected in the margin of the chunks #2213

Open maxjuv opened 1 year ago

maxjuv commented 1 year ago

Hi, I wanted to try the efficiency of a chained peak detection and peak localization. With the same parameters, on the same recording I tried two pipelines (chained vs separated detection/localisation). The chained pipeline crashes at waveform extraction. In fact, some peaks are too close to border.
My code for chained detection/localization, SI version 0.100.dev0 :

extract_dense_waveforms = ExtractDenseWaveforms(recording, ms_before=0.5, ms_after=0.5, return_output=False)
pipeline_nodes = [extract_dense_waveforms,
                 LocalizeMonopolarTriangulation(recording,parents=[extract_dense_waveforms],  **loc_kwargs)]
peaks, peaks_location = detect_peaks(recording,
                                     method='locally_exclusive_torch',
                                     pipeline_nodes = pipeline_nodes ,
                                    **kwargs)

My code for separated detection/localization:

peaks = detect_peaks(recording, method='locally_exclusive_torch', **kwargs)
peaks_location = localize_peaks(recording, peaks,  method='monopolar_triangulation' , **loc_kwargs )

To be noted, for the peak detection I used exclude_sweep_ms=0.1

I tracked the workflow of the node_pipeline and I think the issue comes from the line below. https://github.com/SpikeInterface/spikeinterface/blob/e4c5d11fd58259f7fcb0af41f029a5cac9291e82/src/spikeinterface/core/node_pipeline.py#L508 For peak detection node.get_trace_margin() returns exclude_sweep_ms in samples. If an exclusion sweep is defined, then the margin (called extra_margin) for peak detection in refined. It appeared that peaks can be detected in this extra_margin, which is not a problem if detection and localization are not chained. However, if chained, the peak in the In the case peak localization is chained, the waveforms of the peaks in the extra_margin can no longer be extracted. Finally, if exclude_sweep_ms is set to 0, the chained pipeline workes. With the following observation, I think the issue is located L508 in core.node_pipeline.py

Cheers Maxime

maxjuv commented 1 year ago

Update: In fact with my last run, the separated pipeline also failed, during the peak localization (using torch - cuda). So in the end, I don't know where the bug is. the error is

  File "/home/users/j/juventin/spike_sorters/spikeinterface/src/spikeinterface/core/node_pipeline.py", line 320, in compute
    waveforms = traces[peaks["sample_index"][:, None] + np.arange(-self.nbefore, self.nafter)]
IndexError: index 30030 is out of bounds for axis 0 with size 30030

I asked for a 30000 chunksize (1s) and the margin is 15 samples (.5ms)

maxjuv commented 1 year ago

New update: This time I run the detection (chained or not) with numba (and no longer torch). It works. This suggests the bug is in the DetectPeakLocallyExclusiveTorch ?

alejoe91 commented 1 year ago

Thanks for the report @maxjuv

Definitely looks like a bug with the torch implementation! we'll take a look and add more "border" tests