mne-tools / mne-python

MNE: Magnetoencephalography (MEG) and Electroencephalography (EEG) in Python
https://mne.tools
BSD 3-Clause "New" or "Revised" License
2.72k stars 1.32k forks source link

CSP.plot_filters() is actually plotting the transpose of filters #8579

Closed hongjiang-ye closed 3 years ago

hongjiang-ye commented 3 years ago

Describe the bug

The plot_filters() method of mne.decoding.CSP is not plotting filters, but actually plotting the transpose of filters. Using version 0.21.2.

Steps to reproduce

Use the code from Motor imagery decoding from EEG data using the Common Spatial Pattern (CSP) and change the last line of the second code block csp.plot_patterns to csp.plot_filters, and the result is:

Current plot_filters result

And in mne.decoding.CSP, the each row of filters_ attribute is one CSP filter, see: https://github.com/mne-tools/mne-python/blob/maint/0.21/mne/decoding/csp.py#L184. But if we set the second row of filters_ to be all zeros, the figure almost has no change:

csp.filters_[1, :] = 0  # the coeffs in the second filter(row) are all 0 
csp.plot_filters(epochs.info, ch_type='eeg', units='Patterns (AU)', size=1.5)

Not expected result

Instead, if we set the second column of filters_ to be all zeros, then the "second filter" CSP1 becomes empty:

csp.filters_[:, 1] = 0  # the second column of filters_
csp.plot_filters(epochs.info, ch_type='eeg', units='Patterns (AU)', size=1.5)

empty

So plot_filters is actually plotting the transpose of filters (the columns of filters_), not the csp filters (the rows of filters_).

To fix it

I think just change this line of code https://github.com/mne-tools/mne-python/blob/maint/0.21/mne/decoding/csp.py#L479 to filters = EvokedArray(self.filters_.T, info, tmin=0) could fix this issue. I tried the following code and the figure looks more reasonable to be CSP filters:

csp.filters_ = csp.filters_.T
csp.plot_filters(epochs.info, ch_type='eeg', units='Patterns (AU)', size=1.5)

result

BTW, the figure from plot_patterns method is correct.

cbrnr commented 3 years ago

LGTM. Can you submit a PR?

hongjiang-ye commented 3 years ago

Submitted: #8580. @cbrnr