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.

Sign up for GitHub

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

Use cleaned epochs for sensor-space decoding, time-frequency analysis, and covariance calculation #796

Merged
merged 13 commits into from
Oct 30, 2023
Merged

Use cleaned epochs for sensor-space decoding, time-frequency analysis, and covariance calculation #796

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5837821
Use cleaned epochs for sensor-space decoding
hoechenberger Oct 25, 2023
a88cc46
Always use cleaned epochs in time-frequency and covariance steps
hoechenberger Oct 26, 2023
3e59b19
Add changelog entry
hoechenberger Oct 26, 2023
60bb19b
Update docs/source/v1.5.md.inc
hoechenberger Oct 26, 2023
da9eb4b
Typo
hoechenberger Oct 26, 2023
7a6816c
Fix full-epochs decoding
hoechenberger Oct 30, 2023
0cf3528
FIX: Always warn
larsoner Oct 30, 2023
88357cd
Update mne_bids_pipeline/steps/sensor/_02_decoding_full_epochs.py [ci…
hoechenberger Oct 30, 2023
d674269
Update docs/source/v1.5.md.inc [ci skip]
hoechenberger Oct 30, 2023
5cdc7bf
Merge branch 'main' into use-clean-epochs
hoechenberger Oct 30, 2023
13ae940
Merge branch 'main' into use-clean-epochs
hoechenberger Oct 30, 2023
8475f35
Make it easier to see at which condition CI fails
hoechenberger Oct 30, 2023
642928f
Use 3 CV splits
hoechenberger Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/source/v1.5.md.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## v1.5.0 (unreleased)

This release contains a number of very important bug fixes that address problems related to decoding, time-frequency analysis, and inverse modeling.
All users are encouraged to update.

### :new: New features & enhancements

- Added `deriv_root` argument to CLI (#773 by @vferat)
Expand Down Expand Up @@ -30,3 +33,4 @@
- Fixed bug with parallelization across runs for Maxwell filtering (#761 by @larsoner)
- Fixed bug where head position files were not written with a proper suffix and extension (#761 by @larsoner)
- Fixed bug where default values for `decoding_csp_times` and `decoding_csp_freqs` were not set dynamically based on the config parameters (#779 by @larsoner)
- A number of processing steps erroneously always operated on un-cleaned epochs (`sensor/decoding_full_epochs`, `sensor/decoding_time_by_time`, `sensor/decoding_csp`), or operated on un-cleaned epochs (without PTP rejection) if not ICA or SSP was requested (`sensor/ime_frequency`, `sensor/make_cov`). The bug in `sensor/make_cov` could propagate to the source level as the covariance matrix is used for inverse modeling. (#796 by @hoechenberger)
hoechenberger marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 4 additions & 1 deletion mne_bids_pipeline/steps/sensor/_02_decoding_full_epochs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ def get_input_fnames_epochs_decoding(
run=None,
recording=cfg.rec,
space=cfg.space,
processing="clean",
suffix="epo",
extension=".fif",
datatype=cfg.datatype,
Expand Down Expand Up @@ -94,7 +95,6 @@ def run_epochs_decoding(

epochs = mne.read_epochs(in_files.pop("epochs"))
_restrict_analyze_channels(epochs, cfg)
epochs.crop(cfg.decoding_epochs_tmin, cfg.decoding_epochs_tmax)

# We define the epochs and the labels
if isinstance(cfg.conditions, dict):
Expand All @@ -111,6 +111,9 @@ def run_epochs_decoding(
[epochs[epochs_conds[0]], epochs[epochs_conds[1]]], verbose="error"
)

# Crop to the desired analysis interval.
hoechenberger marked this conversation as resolved.
Show resolved Hide resolved
epochs.crop(cfg.decoding_epochs_tmin, cfg.decoding_epochs_tmax)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any problem with moving this but... how did this fix the failures ?!?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You caught me red-handed there. I believe I'm working around a bug in MNE there. It wouldn't allow me to concatenate epochs where the baseline period was cropped away. So now I first concat, then crop.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's an okay workaround. A comment along those lines would help

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opened an issue: mne-tools/mne-python#12153


n_cond1 = len(epochs[epochs_conds[0]])
n_cond2 = len(epochs[epochs_conds[1]])

Expand Down
1 change: 1 addition & 0 deletions mne_bids_pipeline/steps/sensor/_03_decoding_time_by_time.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ def get_input_fnames_time_decoding(
run=None,
recording=cfg.rec,
space=cfg.space,
processing="clean",
suffix="epo",
extension=".fif",
datatype=cfg.datatype,
Expand Down
5 changes: 1 addition & 4 deletions mne_bids_pipeline/steps/sensor/_04_time_frequency.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ def get_input_fnames_time_frequency(
subject: str,
session: Optional[str],
) -> dict:
processing = None
if cfg.spatial_filter is not None:
processing = "clean"
fname_epochs = BIDSPath(
subject=subject,
session=session,
Expand All @@ -46,7 +43,7 @@ def get_input_fnames_time_frequency(
space=cfg.space,
datatype=cfg.datatype,
root=cfg.deriv_root,
processing=processing,
processing="clean",
suffix="epo",
extension=".fif",
check=False,
Expand Down
1 change: 1 addition & 0 deletions mne_bids_pipeline/steps/sensor/_05_decoding_csp.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ def get_input_fnames_csp(
run=None,
recording=cfg.rec,
space=cfg.space,
processing="clean",
suffix="epo",
extension=".fif",
datatype=cfg.datatype,
Expand Down
3 changes: 1 addition & 2 deletions mne_bids_pipeline/steps/sensor/_06_make_cov.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def get_input_fnames_cov(
) -> dict:
cov_type = _get_cov_type(cfg)
in_files = dict()
processing = "clean" if cfg.spatial_filter is not None else None
fname_epochs = BIDSPath(
subject=subject,
session=session,
Expand All @@ -48,7 +47,7 @@ def get_input_fnames_cov(
space=cfg.space,
extension=".fif",
suffix="epo",
processing=processing,
processing="clean",
datatype=cfg.datatype,
root=cfg.deriv_root,
check=False,
Expand Down
2 changes: 2 additions & 0 deletions mne_bids_pipeline/tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ def pytest_configure(config):
ignore:is_categorical_dtype is deprecated.*:FutureWarning
ignore:use_inf_as_na option is deprecated.*:FutureWarning
ignore:All-NaN axis encountered.*:RuntimeWarning
# sklearn class not enough samples for cv=5
always:The least populated class in y has only.*:UserWarning
"""
for warning_line in warning_lines.split("\n"):
warning_line = warning_line.strip()
Expand Down