[ENH] Add PSD feature via MNE

[anandsaini024]

PSD feature creation script.

[adam2392]

Just to clarify, how are these and the topographic PR different from what is included in ICLabel?

I might've missed this discussion.

[codecov]

Codecov Report

Merging #73 (07888b3) into main (33856df) will increase coverage by 0.08%.
The diff coverage is n/a.

❗ Current head 07888b3 differs from pull request most recent head e1f1d6a. Consider uploading reports for the commit e1f1d6a to get more accurate results

@@            Coverage Diff             @@
##             main      #73      +/-   ##
==========================================
+ Coverage   86.76%   86.84%   +0.08%     
==========================================
  Files          15       16       +1     
  Lines         831      859      +28     
  Branches      101      105       +4     
==========================================
+ Hits          721      746      +25     
- Misses         82       83       +1     
- Partials       28       30       +2     

Impacted Files	Coverage Δ
main/mne_icalabel/utils/_docs.py	93.10% <0.00%> (ø)
main/mne_icalabel/features/psd.py	89.28% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[mscheltienne]

@adam2392 The ICLabel ones are a copy/paste of the MATLAB features.. which are not super clean and nice.
For instance, the ICLabel rpsd feature is supposed to have a random component right? Well.. in practice that's not the case because it does something along the lines of:

some_2d_matrix = [..., ...]
subsets = [indices of all columns]  # instead of an actual subset
select_and_shuffle = shuffle(some_2d_matrix [:, subsets])  # shuffle the columns
average = avg(select_and_shuffle, axis=1)  # avg across columns

And that's not the only example of 'weird' coding choices/behaviors. I added comment in the features.py file describing those items when I ran into them, e.g.

mne-icalabel/mne_icalabel/iclabel/features.py

Lines 375 to 378 in 26a2f1f

	# in MATLAB, 'pct_data' variable is never provided and is always initialized
	# to 100. 'pct_data' is only used in an 'if' statement reached if 'pct_data'
	# is different than 100.. thus, 'pct_data' is not used by this autocorrelation
	# function and is omitted here.

which corresponds to the pct_data variable in this file https://github.com/sccn/ICLabel/blob/d0cfb0f8638cd4518ee2e66a3bff0bb32ff40969/eeg_autocorr_welch.m#L1-L25

variable which is never provided to the function https://github.com/sccn/ICLabel/blob/d0cfb0f8638cd4518ee2e66a3bff0bb32ff40969/ICL_feature_extractor.m#L77

Or this one:

mne-icalabel/mne_icalabel/iclabel/features.py

Lines 426 to 429 in 26a2f1f

	# In MATLAB, 2 scenarios are defined:
	# - EEG.pnts < EEG.srate, which never occurs since then raw provided to
	# this autocorrelation function last at least 5 second.
	# - EEG.pnts > EEG.srate, implemented below.

which corresponds to https://github.com/sccn/ICLabel/blob/d0cfb0f8638cd4518ee2e66a3bff0bb32ff40969/eeg_autocorr_welch.m#L36-L44

---

Overall, I'm not super confident in the ICLabel features, so I asked Anand to prepare functions to extract features from Instance/ICA decomposition using MNE, starting with the topographic map and the PSD based on the ica.plot_properties method. The idea is to use those functions to extract features for future models.

[adam2392]

FYI to fix the style easily and the sorting of imports:

black ./mne_icalabel/* and isort ./mne_icalabel commands, or type make black and make isort. The other checks like flake8 (style), pydocstyle, and mypy can be fixed manually.

[adam2392]

Can we add a better test perhaps? Anything that can be inspired from MNE-Python test suite?

[mscheltienne]

Yes definitely.

[mscheltienne]

@anandsaini024 Please look into additional tests as we discussed:

• test inspired from tests of the PSD functions in MNE
• test on signals with known frequency content, e.g. sinusoid, amplitude modulated signal, ...
• test with different inputs for the arguments of get_psds

I'll try to double-check this PR this week.

[mscheltienne]

@anandsaini024 Please check that the output you are getting from the function makes sense also when you vary the different arguments.
For instance, what happens when you provide a raw which does have "bad" annotations and reject_by_annotation was set to True? Or to False?
And so on.. please test all the arguments in different scenarios to make sure they each work. Those tests that you will do manually at the beginning can then be incorporated in test_psd_arguments.

[mscheltienne]

Please add the match keyword and either part of the error message, the entire error message, or a regex pattern that matches the error. This will avoid catching ANY ValueError.

with pytest.raises(Error, match="error I want to test against"):
    function_that_raises_that_error()

Also, if you notice that an error message is not super clear, then you can start checking the user input at the beginning of the function. I've done it for fmin and fmax, but any error message that does not give clear guidelines to the user on how to correct it is not good enough.

[mmagnuski]

Hi, I popped in to see whats new in mne-icalabel (great project!) and saw your discussion of pct_data variable and how it is not used in the Matlab code. I think (but I may be wrong) that it is related to how EEGLAB usually computed PSD - to save time the spectrum was computed only for a portion of the data (especially important for very long raw recordings and the fact to you expect instant feedback when working via GUI). This option was controlled with a special parameter that in research usage would be set to 100. I think this may be the reason for the behavior of pct_data you see.

[mscheltienne]

Maybe, I didn't dig in too far, but it looked to me like the feature extraction code I translated was half-baked.
And I think the unused pct_data variable appears both in the PSD and autocorrelation features.

Also, I'll finalize this PR at some point ;)