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

Compute and plot PSD for all channels #11722

Closed
cbrnr opened this issue Jun 6, 2023 · 8 comments · Fixed by #12006
Closed

Compute and plot PSD for all channels #11722

cbrnr opened this issue Jun 6, 2023 · 8 comments · Fixed by #12006

Comments

@cbrnr
Copy link
Contributor

cbrnr commented Jun 6, 2023

Our definition of picks is inconsistent across various functions (see e.g. #11382). Here's another example where we could make the behavior more consistent: Plotting the PSD of all channels (good and bad).

Currently, this is not possible, or at least it is not immediately obvious how to do it. I've described this in our forum: https://mne.discourse.group/t/computing-and-plotting-psd-with-all-channels/7025/1

In a nutshell:

  • The only way to include bad channels in psd = raw.compute_psd() is to explicitly list them in picks. Rather unintuitively, picks="all", picks="data", and picks="eeg" all include only good channels.
  • In addition, plotting a Spectrum object with psd.plot() has its own picks parameter. But if bad channels are not present in the first place, the recent change in v1.5 of showing all channels by default has no impact.

Here's a possible solution to improve consistency:

  • Change picks="all" to use all channels (good and bad)
  • Add picks="good" to use only good channels
  • Add picks="bad" to use only bad channels
  • Change picks="data" to use data channels (good and bad)
  • Change picks="eeg" (and other types) to use all channels of that type (good and bad)

However, this gets a bit complicated, especially when considering that at least Spectrum.plot() has an exclude parameter (which is basically the opposite of picks). I assume that only one of the two arguments can be passed, but we'd also need to translate any changes to the behavior of picks to exclude. BTW, raw.compute_psd() has no exclude parameter, which is also inconsistent.

Another option could be to add a new parameter to deal with bad channels (e.g. include_bads=True). This would make the two selections independent of each other (and probably simpler, IDK).

@larsoner
Copy link
Member

larsoner commented Jun 6, 2023

From what I recall in several places we have picks=... paired with exclude=... so you could do for example picks='all', exclude='bads' to pick all non-bad channels, picks='data', exclude=() to pick all good data channels, etc. So maybe all we're missing from some functions (these included) would be an exclude param which says what to exclude as well.

@cbrnr
Copy link
Contributor Author

cbrnr commented Jun 6, 2023

If that's how it's already done in some functions I'm OK with having a combination of picks and exclude. By adding exclude to raw.compute_psd(), we can make the current default behavior explicit (picks="all", exclude="bads") or, even better, make the new default consistent with other places (picks="all", exclude=()).

@cbrnr
Copy link
Contributor Author

cbrnr commented Jun 6, 2023

But then how do we handle things like picks=["C3", "Cz", "C4"], exclude=["C3", "Cz", "C4"]? Do we just take the difference picks - exclude of those sets?

@larsoner
Copy link
Member

larsoner commented Jun 6, 2023

or, even better, make the new default consistent with other places (picks="all", exclude=()).

I think @drammock was actually trying to move this direction recently with some Spectrum class changes so let's wait for him to comment

But then how do we handle things like picks=["C3", "Cz", "C4"], exclude=["C3", "Cz", "C4"]? Do we just take the difference picks - exclude of those sets?

No idea how this is currently handled, exclude might only be used for str-type picks rather than lists. Feel free to check and document somewhere

@agramfort
Copy link
Member

agramfort commented Jun 8, 2023 via email

@drammock
Copy link
Member

Coming back to this one:

  1. raw.compute_psd() doesn't have exclude, and if you pass it picks="all" (or a type string like "eeg") then it will exclude bads silently and automatically. This is expected behavior but not good behavior. It should get an exclude=() kwarg.
  2. Spectrum.pick() has an exclude=() and appears to work correctly (but because of (1) above, you need to manually mark some spectrum channels as bad to see that it works properly --- currently it can never inherit them from the Raw/Epochs/Evoked data source)
  3. Spectrum.plot() doesn't have exclude and has the same problem as .compute_psd(), and should get the same solution (new param exclude=())

I'm pretty sure that exclude=() is the correct default here, rather than exclude="bads".

@larsoner
Copy link
Member

Sounds good to me

@GonReina
Copy link
Contributor

Will be having a go at it!

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 a pull request may close this issue.

5 participants