Skip to content

Conversation

@bparks13
Copy link
Member

To give the widest pass band possible, the DSP cutoff value is now set to Off.

Fixes #543

@bparks13 bparks13 added this to the 0.7.0 milestone Nov 12, 2025
@bparks13 bparks13 requested a review from jonnew November 12, 2025 23:13
@bparks13 bparks13 self-assigned this Nov 12, 2025
@aacuevas
Copy link
Collaborator

I must say, I'm not 100% convinced defaulting the DSP to disabled is the best option
Just as a reminder of what de DSP does: The amplifiers on the Intan chip, past the analog filter, introduce a DC offset that varies with channel. The DSP exists to remove this DC offset and ensuring that all signals are 0-centered. With it disabled, all the signals will appear centered about different values.

Most analysis do their own filtering afterwards, so it is true that in most cases this just adds an unnecessary additional FIR filtering (with all its potential effects). But it will be very noticeable when plotting the signals.

So if we set this to off by default, we need to thoroughly document this.

@jonnew
Copy link
Member

jonnew commented Nov 21, 2025

I understand but having it off is in line with the the least filtering of data by default principle that we tend to follow. e.g. the analog settings are both maxed out.

What does to OE GUI acq. board plugin do by default?

In any case, the text in your comment can simply go in the XML description of this setting!

@aacuevas
Copy link
Collaborator

The GUI defaults to DSP enabled at 0.5Hz and analog filters set at 1.0Hz to 7.5KHz.

I'm ok with this library have defaults that are just the least filtering, I just thought this particular setting needs to be specially clear, since it is not clear at the get-go why is it needed in the first place, or why the data has a DC offset without it.

We might want to throw a line somewhere in the docs about that. But for sure, having this in the xml comments I believe is a good idea and we should!

@bparks13 bparks13 requested a review from aacuevas November 24, 2025 18:04
@bparks13 bparks13 requested a review from aacuevas November 25, 2025 15:06
@bparks13 bparks13 merged commit 7a2895c into main Nov 26, 2025
8 checks passed
@bparks13 bparks13 deleted the issue-543 branch November 26, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default DSP should be set to off in ConfigureRhd2164

4 participants