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
[MRG] Add spectro-spatial decomposition SSD example #7070
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7070 +/- ##
=========================================
- Coverage 89.74% 89.7% -0.05%
=========================================
Files 442 444 +2
Lines 77783 78981 +1198
Branches 12620 12674 +54
=========================================
+ Hits 69807 70850 +1043
- Misses 5167 5319 +152
- Partials 2809 2812 +3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @dengemann for taking a stab at this. How do we integrate this in the MNE public API?
See you can make it embrace the same API as XDawn. Basically have a
transform for arrays and apply for instances. YOu would get filters and
patterns using LinearModel
https://mne.tools/stable/generated/mne.decoding.LinearModel.html#mne.decoding.LinearModel
… |
Another option would be an ICA like api. Allowing for exploring components and handling artifact removal, whitening etc.
You just use a different model to get filters.
This makes me also think that I should explore the alternative whitened eigenvalue problem to compute SSD. It should help us handling magnetometers and gradiometers.
… On 17 Nov 2019, at 16:36, Alexandre Gramfort ***@***.***> wrote:
See you can make it embrace the same API as XDawn. Basically have a
transform for arrays and apply for instances. YOu would get filters and
patterns using LinearModel
https://mne.tools/stable/generated/mne.decoding.LinearModel.html#mne.decoding.LinearModel
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
yes ICA API is similar. fit, transform & apply methods with some plot
methods
… |
@agramfort pushed first API draft, let me know what you think. Todo:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just caught some typos :)
@britta-wstnr @wmvanvliet thank you for your review. Any thoughts on the proposed API? |
filt_params_noise : dict | ||
Filtering for the frequencies of non-interest. | ||
estimator : str | ||
Which covariance estimator to use. Defaults to "oas". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needs pointer to documentation on what estimators you can use and what they do.
+1 for having a |
@wmvanvliet thoughts on apply semantics? It could be used to reconstruct sensor signal from selected components, be it the most or the least oscillatory. We also should have an apply_cov to filter cov matrices. |
Exactly. It could just act as a filter: put an Epochs object in, get an Epochs object out that was filtered for maximum oscillatory activity. Put a Covariance object in, get a Covariance object out (no need for an explicit |
cov_signal = mne.compute_raw_covariance( | ||
inst_signal, picks=self.picks_, method=self.estimator) | ||
cov_noise = mne.compute_raw_covariance(inst_noise, picks=self.picks_, | ||
method=self.estimator) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder to self: expose flat/reject values.
Hi, if I understand it correctly this is the same GED that is used internally in common spatial pattern and that also has multiple other applications for EEG/MEG (see for example Mike X Cohen's publications on GED). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi I have been playing around and cheking this fuction implementation. I have some suggestions
else: | ||
raise NotImplementedError() | ||
|
||
self.eigvals_, self.filters_ = eigh(cov_signal.data, cov_noise.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here the filters are sorted by eigvals ascending order, could it be more helpful to haven them already sorted in descending order?
self.eigvals_, self.filters_ = eigh(cov_signal.data, cov_noise.data) | |
self.eigvals_, self.filters_ = eigh(cov_signal.data, cov_noise.data) | |
#sort in descencing order | |
ix = np.argsort(eigvals_)[::-1] | |
self.eigvals_=eigvals_[ix] | |
self.filters_ = eigvects_[:, ix] |
sptial filter. | ||
sort_by_spectral_ratio : bool | ||
Whether to sort by the spectral ratio. Defaults to True. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this input is not used then, since I guess what was the goal of such input variable, I suggest how to use it while transform is called.
|
||
self.eigvals_, self.filters_ = eigh(cov_signal.data, cov_noise.data) | ||
self.patterns_ = np.linalg.pinv(self.filters_) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here I used the function spectral_ratio_ssd provided in the example within SSD to be called during ssd.transform
def spectral_ratio_ssd(self,ssd_sources): | |
"""Spectral ratio measure for best n_components selection | |
See Nikulin 2011, Eq. (24) | |
""" | |
psd, freqs = mne.time_frequency.psd_array_welch( | |
ssd_sources, sfreq=self.sfreq,n_fft=4096) | |
sig_idx = freq_mask(freqs, *self.freqs_signal) | |
noise_idx = freq_mask(freqs, *self.freqs_noise) | |
spec_ratio = psd[:, sig_idx].mean(axis=1) / psd[:, noise_idx].mean(axis=1) | |
sorter_spec = spec_ratio.argsort()[::-1] | |
return spec_ratio, sorter_spec |
|
||
spec_ratio = spectral_ratio_ssd(psd, freqs, freqs_sig, freqs_noise) | ||
|
||
sorter = spec_ratio.argsort() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also this guy
sorter = spec_ratio.argsort() | |
sorter = ssd.sorter_spec |
Thank you @vpeterson for your review, I shall get back to you during this week. I have marked this PR as a potential candidate for our next release, hoping that we can merge a first version within the next 3 weeks. |
Have any time for this @dengemann ? If not let's push to 0.22 |
Yes I will get to it this week.
… On 9 Sep 2020, at 16:25, Eric Larson ***@***.***> wrote:
Have any time for this @dengemann ? If not let's push to 0.22
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
@vpeterson I rebased the pull request to allow us to continue working on this. Shall we give it another try to let you do a PR into this one? We can work through the necessary steps. It would be nice to give credit to your work through commits. I'd also have few discussion points regarding the code you have shared with me. |
Other than this PR things are looking on track for release tomorrow |
should be good now, carried forward your change @vpeterson |
Sorry @dengemann @vpeterson , one more rebase necessary because of |
ok looking into that shortly
… On 12 Nov 2020, at 18:00, Eric Larson ***@***.***> wrote:
Sorry @dengemann @vpeterson , one more rebase necessary because of latest.inc :(
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
cosmits cosmits draft API added option to handle epoched data, sort component with respect to spectral information and implement an apply function to go back to the signal space Big changes: 1. handle both raw and epoched data. 2. added a boolean input which allows for the components to be sorted with respect to the spectral information. 3. added the apply function, to back project into the signal space Minnor changes: 1. The optinal subtract_signal input was deleted, since this should always be done for estimating the noise. 2. Added a band-fill to the picks plot to highlight the freq-band of interest update version, still missing: bib Update plot_ssd_spatial_filters.py style-checker passed, added check_input and filter_data added :footcite: added link to funcs and handle Epochs Update plot_ssd_spatial_filters.py backticks for references integrate work by @vpeterson + bugfixes + enhance API Update examples/decoding/plot_ssd_spatial_filters.py Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> typos make info keyword, handle channel types
clean up sorting revert default move SSD to decoding module add SSD work in progress test on raw and simulated data changes at transform and apply in .transform solved the boolean bug when sort_by_spectral_power was set to False. .apply change to .inverse_transform clean pick variable (no usage anymore) picks was being used! back to it update test_ssd.py added true components detection, error between true and estimated mixing matrix, skelearn pipeline and raw vs epoch analysis update version acording to comments solving code style, run pipeline, add channels type update requirements files added white space added changes accroding to pytest fails Update version according to comments version with pytest passed small changes code style added picks documentation add reference changes according to tests typos checked by codespell-error Apply suggestions from code review Co-authored-by: Eric Larson <larson.eric.d@gmail.com> check documentation building avoid usage of private functions change cross-ref and add __hash__ added decoding.SSD to conf.py file
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
Rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One remaining issue with the existing code, otherwise LGTM!
We should probably add it to tutorials/machine-learning/plot_sensors_decoding.py, though, since that is more or less our directory of machine learning techniques. Can you add a section in there? If it's easy to quickly run on sample
data and it makes sense to do so feel free, otherwise just add a new section like we have for Xdawn linking to the new example.
@larsoner the idea is to merge the basics here that provide a solid foundation for doing SSD. There is more work planned and we want for sure a good machine learning example. I would love to do this in several PRs though. Ok if we postpone additional examples? |
I can push it quickly if you are both burned out on making changes -- I think for each new ML method it should have a corresponding entry in |
Not sure if we need to rush with this. But if you think we need this before merge I don't object if you try the code. One could add it as preprocessor to SPoC or CSP. But I'd really like not to rush with this. We still have some time in this release cycle. |
Ahh okay I was thinking about it more as a method meant to be used directly like Xdawn. We can add it later when the bigger picture is clearer then if necessary |
It's actually unsupervised, if you want. It's more like SSP/ICA actually. I see two important use cases, boosting SNR in noisy data and adding specificity to the interpretation of results as you can kick out non-oscillatory components or signal from surrounding bands. Exploring good examples might take some time. If you plug it in as is you may or may not see obvious benefits for prediction performance. |
I agree with this. I just add a comment, in the test_ssd.py we also test how SSD can be used before CSP within the makepipeline sklearn structure. So, yeah, future improvement will be to show the usage of SSD in a good example, but it may take some time. |
One Azure build is timing out, unrelated failure. |
now all green @dengemann !! |
Thanks @vpeterson @dengemann ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry I was slow on this one
@dengemann can you have a look?
happy to discuss if you want
thanks for this cool feature
@@ -0,0 +1,138 @@ | |||
""" | |||
=========================================================== | |||
Compute Sepctro-Spatial Decomposition (SDD) spatial filters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SDD -> SSD
|
||
ssd = SSD(info=raw.info, | ||
reg='oas', | ||
sort_by_spectral_ratio=False, # True is recommended here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you use False and in the recommended way you say it should be True?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need it below for the purpose of the example.
n_components : int | None (default None) | ||
The number of components to extract from the signal. | ||
If n_components is None, no dimensionality reduction is applied, and | ||
the transformed data is projected in the whole source space. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
misnomer, indeed. should be fixed.
picks : array of int | None (default None) | ||
The indices of good channels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes !
|
||
Attributes | ||
---------- | ||
filters_ : array, shape(n_channels, n_components) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
space missing before (
self.freqs_signal = (filt_params_signal['l_freq'], | ||
filt_params_signal['h_freq']) | ||
self.freqs_noise = (filt_params_noise['l_freq'], | ||
filt_params_noise['h_freq']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why storing freqs_signal and freqs_noise as it's already in the filt_params_signal and filt_params_noise attributes?
|
||
return self | ||
|
||
def transform(self, X, y=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please no y in transform
obtained from continuous data or 3D array obtained from epoched | ||
data. | ||
y : None | array, shape (n_samples,) | ||
Used for scikit-learn compatibility. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is no y in transform in sklearn
raise NotImplementedError('See ssd.inverse_transform()') | ||
|
||
def inverse_transform(self, X): | ||
"""Remove selected components from the signal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not an inverse transform
if you follow the API of ICA apply will denoise the data and transform will bring you into "source space"
this deviates from our CSP API
Data are simulated in the statistical source space, where n=n_components | ||
sources contain the peak of interest. | ||
""" | ||
rs = np.random.RandomState(random_state) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we call it rng usually
This is a first example demonstrating spatial filtering with SSD (Nikulin et al 2011, NIMG), which essentially enhances the oscillatory signal of interest by removing background signal from surrounding frequencies.
The example is autonomous as is, but if we converge on ideas, I'd extend it to have a Transformer API like SPoC or CSP.
cc @agramfort @DavidSabbagh @pierreablin @wmvanvliet @britta-wstnr