SpikeInterface / spikeinterface

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

detect_peaks method returns peaks in neighboring channels when set to 'locally_exclusive' #1929

Open lavanv1107 opened 1 year ago

lavanv1107 commented 1 year ago

I am currently using SpikeInterface's peak detection module to detect peaks from an NWB recording that I have.

detect_peaks(recording, method='locally_exclusive', peak_sign='neg', detect_threshold=6, exclude_sweep_ms=0.1, **job_kwargs )

This returns peaks which occur at the same sample_ind and neighboring channels (353 and 354). Channel 353 (48, 3520) and Channel 354 (0, 3540) are 52 um apart. This is less than 100 um, which is the default exclusion radius specified.

This seems abnormal. Do you know why this might be?

cc @rly

h-mayorquin commented 1 year ago

Probably a bug, the detect_peaks module is not really well tested. I remember that if there was a problem with detection at the chunks borders. Probably needs a deep dive into the implementation.

alejoe91 commented 1 year ago

@lavanv1107 We'll have to wait for @samuelgarcia to be back from vacation next week. Did you find many of these "mistakes"?

h-mayorquin commented 1 year ago

@lavanv1107 One more thing, can you show us a plot (showing the signal) of the area where the peaks are wrongly not excluded. This will be useful for us to generate minimal example / testing on the future.

samuelgarcia commented 1 year ago

Hi @lavanv1107. Thanks for reporting this. This look like a very sad bug. Could you send us a piece of data to check this ?

Be aware that the output is a peak vector and the peak vector have a "channel_index" which is NOT the channel id. You have to go back to channel id with channel_ids[channel_index]. Can you check that you are not mixing channel ID and index when refering to 353 and 354 and there location.

lavanv1107 commented 9 months ago

Hi everyone,

Sorry for the late response. I decided to work on other parts of my project at the time and did not follow up on the issue. However, I've encountered it again in my work.

I've attached materials for testing at Google Drive. It includes:

@alejoe91 Yes, there are many of them. With the peak vector I obtained, there are 3939 "neighbor" peaks.

@h-mayorquin I've added a method which plots signals for these neighbor peaks. I've created example plots for different groups of them.

@samuelgarcia I checked the channel_ids object, and it looks to me that the channel_indices from the "channel_index" column of the peak vector are the same. I've also included this in the notebook as well.

rly commented 9 months ago

Looking through the code with @lavanv1107 , I suspect that, even in locally exclusive mode, when the signals from multiple neighboring channels cross the threshold at the same time, they are all returned, even when we set the radius_um to be very large, e.g., larger than the size of the probe. Is this behavior expected? We had expected "locally exclusive" to have filtered out peaks from channels within radius_um already, keeping the peak with the largest amplitude, perhaps, but we haven't found code that does that.

alejoe91 commented 9 months ago

@samuelgarcia we should take a look at this soon

JoeZiminski commented 4 months ago

I want to understand more the peak detection modules so will assign this (not exclusively) to myself

h-mayorquin commented 4 months ago

I also need to work on peak detection later this summer so I can take a stab.