From 8a76518845ccee507b2d74225bba07cff0b4c444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Richard=20H=C3=B6chenberger?= Date: Thu, 18 Apr 2024 12:02:20 +0200 Subject: [PATCH 1/5] Don't perform ICA cleaning on raw data by default when epochs are present Fixes #925 --- docs/source/v1.9.md.inc | 11 +++++++++-- mne_bids_pipeline/_config.py | 7 +++++++ .../steps/preprocessing/_08a_apply_ica.py | 9 ++++++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/docs/source/v1.9.md.inc b/docs/source/v1.9.md.inc index 39103483c..dbd33e27c 100644 --- a/docs/source/v1.9.md.inc +++ b/docs/source/v1.9.md.inc @@ -2,15 +2,22 @@ ### :new: New features & enhancements -- Added number of subject to `sub-average` report (#902, #910 by @SophieHerbst) +- Added number of subjects to `sub-average` report (#902, #910 by @SophieHerbst) - The type annotations in the default configuration file are now easier to read: We replaced `Union[X, Y]` with `X | Y` and `Optional[X]` with `X | None`. (#908, #911 by @hoechenberger) +- Added a new configuration option [`spatial_filter_raw`][mne_bids_pipeline._config.spatial_filter_raw] + to control whether to create cleaned raw data via ICA or SSP. Previously, we'd only clean epochs. + (#840, #926 by @larsoner and @hoechenberger) ### :warning: Behavior changes - All ICA HTML reports have been consolidated in the standard subject `*_report.html` file instead of producing separate files (#899 by @larsoner). -- Changed default for `source_info_path_update` to `None`. In `_04_make_forward.py` +- When using ICA or SSP with resting-state data, we now automatically produce cleaned raw data files. This + behavior can be controlled via the new [`spatial_filter_raw`][mne_bids_pipeline._config.spatial_filter_raw] + configuration option. (#840, #926 by @larsoner and @hoechenberger) +- Changed default for [`source_info_path_update`][mne_bids_pipeline._config.source_info_path_update] + to `None`. In `_04_make_forward.py` and `_05_make_inverse.py`, we retrieve the info from the file from which the `noise_cov` is computed (#919 by @SophieHerbst) - The [`depth`][mne_bids_pipeline._config.depth] parameter doesn't accept `None` diff --git a/mne_bids_pipeline/_config.py b/mne_bids_pipeline/_config.py index 912d257f0..adb460c31 100644 --- a/mne_bids_pipeline/_config.py +++ b/mne_bids_pipeline/_config.py @@ -1138,6 +1138,13 @@ ICA fitting – this file will need to be updated! """ +spatial_filter_raw: bool | None = None +""" +Whether to clean the raw data with the spatial filter. If `True`, creates cleaned data +from both, epochs and raw. If `False`, only creates cleaned epochs. If `None`, creates +cleaned epochs and raw for resting-state data, and only cleaned epochs otherwise. +""" + min_ecg_epochs: Annotated[int, Ge(1)] = 5 """ Minimal number of ECG epochs needed to compute SSP projectors. diff --git a/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py b/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py index 5b097c106..b23b2314c 100644 --- a/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py +++ b/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py @@ -196,7 +196,12 @@ def apply_ica_raw( run: str, task: str | None, in_files: dict, -) -> dict: +) -> dict | None: + if cfg.spatial_filter_raw is None and not (cfg.task == "rest" or cfg.task_is_rest): + return + elif cfg.spatial_filter_raw is False: + return + ica = _read_ica_and_exclude(in_files) in_key = list(in_files)[0] assert in_key.startswith("raw"), in_key @@ -240,6 +245,8 @@ def get_config( cfg = SimpleNamespace( baseline=config.baseline, ica_reject=config.ica_reject, + spatial_filter_raw=config.spatial_filter_raw, + task_is_rest=config.task_is_rest, processing="filt" if config.regress_artifact is None else "regress", _epochs_split_size=config._epochs_split_size, **_import_data_kwargs(config=config, subject=subject), From 2887d8fd9fd435b81b17049bfe586ad1e6adfafd Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Thu, 18 Apr 2024 14:58:38 -0400 Subject: [PATCH 2/5] FIX: Simplify --- mne_bids_pipeline/_config.py | 4 ++-- mne_bids_pipeline/_config_utils.py | 9 +++++++++ .../steps/preprocessing/_08a_apply_ica.py | 12 ++++-------- .../steps/preprocessing/_08b_apply_ssp.py | 3 +++ 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/mne_bids_pipeline/_config.py b/mne_bids_pipeline/_config.py index adb460c31..ee43fff95 100644 --- a/mne_bids_pipeline/_config.py +++ b/mne_bids_pipeline/_config.py @@ -1141,8 +1141,8 @@ spatial_filter_raw: bool | None = None """ Whether to clean the raw data with the spatial filter. If `True`, creates cleaned data -from both, epochs and raw. If `False`, only creates cleaned epochs. If `None`, creates -cleaned epochs and raw for resting-state data, and only cleaned epochs otherwise. +for all raw data. If `False`, no raw data is written. If `None`, creates +cleaned raw data for resting-state and empty-room runs if present. """ min_ecg_epochs: Annotated[int, Ge(1)] = 5 diff --git a/mne_bids_pipeline/_config_utils.py b/mne_bids_pipeline/_config_utils.py index 46990a623..f794f9a07 100644 --- a/mne_bids_pipeline/_config_utils.py +++ b/mne_bids_pipeline/_config_utils.py @@ -658,3 +658,12 @@ def _proj_path( suffix="proj", check=False, ) + + +def _which_spatial_filter_raw(*, config: SimpleNamespace) -> tuple[str]: + if config.spatial_fliter_raw: + return ("runs", "noise", "rest") + elif config.spatial_filter_raw is None: + return ("noise", "rest") + else: + return () diff --git a/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py b/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py index b23b2314c..0e6ab9b0a 100644 --- a/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py +++ b/mne_bids_pipeline/steps/preprocessing/_08a_apply_ica.py @@ -13,6 +13,7 @@ from mne_bids import BIDSPath from ..._config_utils import ( + _which_spatial_filter_raw, get_runs_tasks, get_sessions, get_subjects, @@ -196,12 +197,7 @@ def apply_ica_raw( run: str, task: str | None, in_files: dict, -) -> dict | None: - if cfg.spatial_filter_raw is None and not (cfg.task == "rest" or cfg.task_is_rest): - return - elif cfg.spatial_filter_raw is False: - return - +) -> dict: ica = _read_ica_and_exclude(in_files) in_key = list(in_files)[0] assert in_key.startswith("raw"), in_key @@ -245,8 +241,6 @@ def get_config( cfg = SimpleNamespace( baseline=config.baseline, ica_reject=config.ica_reject, - spatial_filter_raw=config.spatial_filter_raw, - task_is_rest=config.task_is_rest, processing="filt" if config.regress_artifact is None else "regress", _epochs_split_size=config._epochs_split_size, **_import_data_kwargs(config=config, subject=subject), @@ -283,6 +277,7 @@ def main(*, config: SimpleNamespace) -> None: parallel, run_func = parallel_func( apply_ica_raw, exec_params=config.exec_params ) + which = _which_spatial_filter_raw(config=config) logs += parallel( run_func( cfg=get_config( @@ -301,6 +296,7 @@ def main(*, config: SimpleNamespace) -> None: config=config, subject=subject, session=session, + which=which, ) ) save_logs(config=config, logs=logs) diff --git a/mne_bids_pipeline/steps/preprocessing/_08b_apply_ssp.py b/mne_bids_pipeline/steps/preprocessing/_08b_apply_ssp.py index ee2c56cb9..38ae2ca4f 100644 --- a/mne_bids_pipeline/steps/preprocessing/_08b_apply_ssp.py +++ b/mne_bids_pipeline/steps/preprocessing/_08b_apply_ssp.py @@ -10,6 +10,7 @@ from ..._config_utils import ( _proj_path, + _which_spatial_filter_raw, get_runs_tasks, get_sessions, get_subjects, @@ -181,6 +182,7 @@ def main(*, config: SimpleNamespace) -> None: parallel, run_func = parallel_func( apply_ssp_raw, exec_params=config.exec_params ) + which = _which_spatial_filter_raw(config=config) logs += parallel( run_func( cfg=get_config( @@ -199,6 +201,7 @@ def main(*, config: SimpleNamespace) -> None: config=config, subject=subject, session=session, + which=which, ) ) save_logs(config=config, logs=logs) From 447f0c0f4924bbc35e89f22c3c71ee212e4340db Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Thu, 18 Apr 2024 15:09:41 -0400 Subject: [PATCH 3/5] FIX: main --- mne_bids_pipeline/tests/test_documented.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/mne_bids_pipeline/tests/test_documented.py b/mne_bids_pipeline/tests/test_documented.py index a6275a4c8..5de599c45 100644 --- a/mne_bids_pipeline/tests/test_documented.py +++ b/mne_bids_pipeline/tests/test_documented.py @@ -76,6 +76,9 @@ def test_config_options_used(): config_names.add(key) for key in _EXECUTION_OPTIONS: config_names.remove(key) + # Used in main only + for key in ("spatial_filter_raw",): + config_names.remove(key) pcs = _ParseConfigSteps(force_empty=()) missing_from_config = sorted(set(pcs.steps) - config_names) assert missing_from_config == [], f"Missing from config: {missing_from_config}" From f5b5f062c2e750b4ef567f79128af9af70313f5a Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Thu, 18 Apr 2024 15:28:22 -0400 Subject: [PATCH 4/5] FIX: spell --- mne_bids_pipeline/_config_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mne_bids_pipeline/_config_utils.py b/mne_bids_pipeline/_config_utils.py index f794f9a07..70b0f1b5f 100644 --- a/mne_bids_pipeline/_config_utils.py +++ b/mne_bids_pipeline/_config_utils.py @@ -661,7 +661,7 @@ def _proj_path( def _which_spatial_filter_raw(*, config: SimpleNamespace) -> tuple[str]: - if config.spatial_fliter_raw: + if config.spatial_filter_raw: return ("runs", "noise", "rest") elif config.spatial_filter_raw is None: return ("noise", "rest") From e9b76c65b1cf351face409da8376f8331c1a34f8 Mon Sep 17 00:00:00 2001 From: Eric Larson Date: Thu, 18 Apr 2024 16:19:38 -0400 Subject: [PATCH 5/5] FIX: Better parsing --- mne_bids_pipeline/_docs.py | 7 ++++++- mne_bids_pipeline/tests/test_documented.py | 3 --- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/mne_bids_pipeline/_docs.py b/mne_bids_pipeline/_docs.py index 440b34690..759d5e2aa 100644 --- a/mne_bids_pipeline/_docs.py +++ b/mne_bids_pipeline/_docs.py @@ -100,6 +100,7 @@ "_bids_kwargs": ("get_task",), "_import_data_kwargs": ("get_mf_reference_run",), "get_runs": ("get_runs_all_subjects",), + "_which_spatial_filter_raw": ("spatial_filter_raw",), } @@ -144,6 +145,10 @@ def __init__(self, force_empty=None): for call in ast.walk(func): if not isinstance(call, ast.Call): continue + # e.g., which = _which_spatial_filter_raw(config=config) + if isinstance(call.func, ast.Name): + for attr in _EXTRA_FUNCS.get(call.func.id, ()): + self._add_step_option(step, attr) for keyword in call.keywords: if not isinstance(keyword.value, ast.Attribute): continue @@ -226,7 +231,7 @@ def __init__(self, force_empty=None): if isinstance(keyword.value, ast.Name): key = f"{where}:{keyword.value.id}" if key in _MANUAL_KWS: - for option in _MANUAL_KWS[f"{where}:{keyword.value.id}"]: + for option in _MANUAL_KWS[key]: self._add_step_option(step, option) continue raise RuntimeError(f"{where} cannot handle Name {key=}") diff --git a/mne_bids_pipeline/tests/test_documented.py b/mne_bids_pipeline/tests/test_documented.py index 5de599c45..a6275a4c8 100644 --- a/mne_bids_pipeline/tests/test_documented.py +++ b/mne_bids_pipeline/tests/test_documented.py @@ -76,9 +76,6 @@ def test_config_options_used(): config_names.add(key) for key in _EXECUTION_OPTIONS: config_names.remove(key) - # Used in main only - for key in ("spatial_filter_raw",): - config_names.remove(key) pcs = _ParseConfigSteps(force_empty=()) missing_from_config = sorted(set(pcs.steps) - config_names) assert missing_from_config == [], f"Missing from config: {missing_from_config}"