Fix bug spatial filter #4175#4286
Conversation
src/spikeinterface/preprocessing/tests/test_highpass_spatial_filter.py
Outdated
Show resolved
Hide resolved
src/spikeinterface/preprocessing/tests/test_highpass_spatial_filter.py
Outdated
Show resolved
Hide resolved
| si_recording = se.read_spikeglx(local_path, stream_id="imec0.ap") | ||
| si_recording = spre.astype(si_recording, "float") | ||
| recording_ps = si.phase_shift(si_recording) | ||
| recording_hp = si.highpass_filter(recording_ps, freq_min=300, filter_order=3) |
There was a problem hiding this comment.
| recording_hp = si.highpass_filter(recording_ps, freq_min=300, filter_order=3) | |
| recording_hp = spre.highpass_filter(recording_ps, freq_min=300, filter_order=3) |
src/spikeinterface/preprocessing/tests/test_highpass_spatial_filter.py
Outdated
Show resolved
Hide resolved
|
|
||
|
|
||
| def agc(traces, window, epsilon=1e-8): | ||
| def agc(traces, window, epsilon=None): |
There was a problem hiding this comment.
I think this is a problem here because the espilon will change for every chunk no ?
There was a problem hiding this comment.
I would vote to force espilon here because it is an helper function and compute it the init of the class.
no ?
There was a problem hiding this comment.
Yes but this is a second order issue, we just need to stabilize the inverse of the gain.
To mitigate, I have exposed the epsilon as a parameter in the interface. It would be useful to have a BaseRecording.estimate_rms() method to fix a single epsilon for the full processing, but this is beyond the scope of this PR.
If everyone is on-board for this, I can create an issue and summarize this discussion
There was a problem hiding this comment.
Sounds good to me. We hav an get_noise_levels(recording, method="rms") that we could run in the init if rms levels are not estimated yet. But let's discuss specifics on the new issue
| num_channels_per_adc = 12 | ||
|
|
||
| sample_shifts = get_neuropixels_sample_shifts(self.get_num_channels(), num_channels_per_adc) | ||
| sample_shifts = get_neuropixels_sample_shifts_from_probe(probe, num_channels_per_adc) |
There was a problem hiding this comment.
fixed this, which now needs to be probe directly!
There was a problem hiding this comment.
good with me, I also fixed the tests so the CI passes.
|
ok for me also. |
4 participants
Fixed the main bug: all of the processing has to be done in float, even if only to reconvert to int at the end of the module.
I have also improved a few things:
neurodspmodule doesn't exist anymore