ENH: Add artifact regression #837
Conversation
larsoner
changed the title
| @@ -449,7 +449,6 @@ def import_er_data( | |||
| cfg=cfg, | |||
| bids_path_bads=bids_path_er_bads_in, | |||
| ) | |||
| raw_er.pick("meg", exclude=[]) | |||
There was a problem hiding this comment.
For some reason we were only picking MEG channels in empty room data. To me this step seems unnecessary, and also breaks artifact regression, so I think we should drop it.
| msg = "Adding config and sys info to report" | ||
| logger.info(**gen_log_kwargs(message=msg)) |
There was a problem hiding this comment.
Moved this later in the file where we can make sure we emit it exactly once per mne_bids_pipeline call (other calls should be fast and it's not helpful to have dozens of these messages in the logs)
| subject=subject, # save under subject's directory so all files are there | ||
| session=session, |
There was a problem hiding this comment.
Previously the filt step for SSS processing would output to sub-01/* type paths, whereas if SSS was off it would output to sub-emptyroom/* type paths. This unifies it so everything lands in subject-specific paths. This means that after the filt step (e.g., during regression, SSP, and ICA) we can assume all files for each subject come from that subject's path rather than potentially sub-emptyroom. This may mean a few redundant files but simplifies logic and should help with #831.
| @@ -199,6 +204,18 @@ def filter_data( | |||
| check=False, | |||
| ) | |||
|
|
|||
| if cfg.regress_artifact is None: | |||
There was a problem hiding this comment.
Need to filter the data and regression channels the same way
|
|
||
| _STEPS = ( | ||
| _01_data_quality, | ||
| _02_head_pos, | ||
| _03_maxfilter, | ||
| _04_frequency_filter, | ||
| _05_make_epochs, | ||
| _05_regress_artifact, |
There was a problem hiding this comment.
Previously the order was SSS, filt, epoch, SSP/ICA, clean epochs. Now the order is SSS, filt, SSP/ICA, epoch, clean epochs. This makes it clearer that the epochs are not used for SSP and ICA, which is helpful in terms of understanding the flow in that we basically stay working with raw data ountil step _07 now.
| @@ -202,7 +213,7 @@ | |||
| covariance (via `noise_cov='rest'`). | |||
| """ | |||
|
|
|||
| ch_types: Iterable[Literal["meg", "mag", "grad", "eeg"]] = [] | |||
| ch_types: Annotated[Sequence[Literal["meg", "mag", "grad", "eeg"]], Len(1, 4)] = [] | |||
There was a problem hiding this comment.
I errantly had ch_types = "meg". By changing this from Iterable to Sequence, I got a nice error message saying it couldn't just be a string. Then we can use pydantic to further validate the length such as non-emptiness using Len from annotated_types, which is required by pydantic. So the deletions in _config_import.py below are now handled by pydantic for us!
.circleci/config.yml
Outdated
| @@ -48,7 +48,7 @@ jobs: | |||
| name: Get Python running | |||
| command: | | |||
| pip install --upgrade --progress-bar off pip | |||
| pip install --upgrade --progress-bar off "autoreject @ https://api.github.com/repos/autoreject/autoreject/zipball/master" "mne[hdf5] @ git+https://github.com/mne-tools/mne-python@main" "mne-bids[full] @ https://api.github.com/repos/mne-tools/mne-bids/zipball/main" numba | |||
| pip install --upgrade --progress-bar off "autoreject @ https://api.github.com/repos/autoreject/autoreject/zipball/master" "mne[hdf5] @ git+https://github.com/larsoner/mne-python@regress" "mne-bids[full] @ https://api.github.com/repos/mne-tools/mne-bids/zipball/main" numba | |||
There was a problem hiding this comment.
Temporarily use my branch until mne-tools/mne-python#12389 lands
|
... @hoechenberger I think this is ready for review. If you're happy I'll wait until mne-tools/mne-python#12389 lands, revert the change to use my branch, then merge |
|
Seems to work, nice low-freq noise suppressed by regressing the ref sensors! I'll work on fixing the conflicts then should be good to go |
|
Thanks @larsoner! |
Before merging …
docs/source/changes.md)Adds
config.regress_artifact, used in my pipeline to regress reference sensor signals:Draft because I need to find a good KIT dataset to process to show this stuff, any ideas @agramfort ? I saw MASC-MEG but it's continuous speech (which we probably don't handle well) and huge.
I might try converting https://mne.tools/dev/auto_tutorials/inverse/95_phantom_KIT.html to BIDS since it works to demonstrate the processing.
Closes #832