Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in GaussianFilter #2466

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Fix bug in GaussianFilter #2466

merged 2 commits into from
Feb 5, 2024

Conversation

DradeAW
Copy link
Contributor

@DradeAW DradeAW commented Feb 5, 2024

Following up on the discussion we had in #2397

Indeed there is a bug in the Gaussian filter implementation, which matters if freq_min and freq_max are close to one another, or even if the min is above the max

Here is a quick code to check for the bug:

import numpy as np
import plotly.graph_objects as go
import spikeinterface.core as si
import spikeinterface.preprocessing as spre


sf = 30_000.0


# An impulse contains all frequencies equally.
trace = np.zeros([int(sf), 1], dtype=np.float64)
trace[15000] = 1.0

recording = si.NumpyRecording(trace, sf)
recording_f = spre.gaussian_filter(recording, freq_min=600, freq_max=300)

trace_f = recording_f.get_traces()


fig = go.Figure()

fig.add_trace(go.Scatter(
	x=1e3 * np.arange(sf) / sf,
	y=trace[:, 0],
	mode="lines",
	name="Original trace"
))
fig.add_trace(go.Scatter(
	x=1e3 * np.arange(sf) / sf,
	y=trace_f[:, 0],
	mode="lines",
	name="Filtered trace"
))

fig.update_xaxes(title_text="Time (ms)")

fig.show()

Here is what the plot looks like before the patch:
bug_gaussian_before

And what is looks like after this patch:
bug_gaussian_after

We can see the impulse is no longer going in the other direction, and there is less signal overall (which is expected)
Thanks @TomBugnon for pointing out the issue!

@alejoe91 alejoe91 added this to the 0.100.0 milestone Feb 5, 2024
@alejoe91 alejoe91 added bug Something isn't working preprocessing Related to preprocessing module labels Feb 5, 2024
@alejoe91 alejoe91 merged commit 6bb7e71 into SpikeInterface:main Feb 5, 2024
11 checks passed
@DradeAW DradeAW deleted the patch-1 branch March 20, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working preprocessing Related to preprocessing module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants