mne-tools / mne-bids-pipeline

Automatically process entire electrophysiological datasets using MNE-Python.
https://mne.tools/mne-bids-pipeline/
BSD 3-Clause "New" or "Revised" License
137 stars 65 forks source link

Add support for `autoreject` (local) before running ICA #810

Closed hoechenberger closed 10 months ago

hoechenberger commented 10 months ago

Before merging …

hoechenberger commented 10 months ago

Report:

https://output.circle-artifacts.com/output/job/586ce7bb-89b6-466c-84ab-23277c1ce14c/artifacts/0/reports/ERP_CORE/sub-015_ses-ERN_task-ERN_proc-ica+components_report.html#Epochs__Autoreject_cleaning

Tests are still taking too long, will look into this before we merge

larsoner commented 10 months ago

Do you need both local and global options at both stages (before and after ICA), or can we get away with / is it advisable to use one or the other at one stage vs the other? Even if it's easy to add both options both places, if it's unlikely (or typically unwise) to use one or the other then it might be better to omit it for now and add it only when someone needs it.

Is there any autoreject doc / tutorial we can link to in order to help people decide?

hoechenberger commented 10 months ago

Do you need both local and global options at both stages (before and after ICA), or can we get away with / is it advisable to use one or the other at one stage vs the other? Even if it's easy to add both options both places, if it's unlikely (or typically unwise) to use one or the other then it might be better to omit it for now and add it only when someone needs it.

Is there any autoreject doc / tutorial we can link to in order to help people decide?

For ICA, the recommended way is using AR local before ICA (on the epochs that will be used for fitting), then clean the data with ICA, and running AR local again, this time on those cleaned epochs

I think we didn't add AR global for pre-ICA cleaning in the past because it would reject just too many epochs. I could remove that option

hoechenberger commented 10 months ago

Official docs on AR & ICA: https://autoreject.github.io/dev/auto_examples/plot_autoreject_workflow.html

hoechenberger commented 10 months ago

Do you need both local and global options at both stages (before and after ICA), or can we get away with / is it advisable to use one or the other at one stage vs the other? Even if it's easy to add both options both places, if it's unlikely (or typically unwise) to use one or the other then it might be better to omit it for now and add it only when someone needs it. Is there any autoreject doc / tutorial we can link to in order to help people decide?

For ICA, the recommended way is using AR local before ICA (on the epochs that will be used for fitting), then clean the data with ICA, and running AR local on those cleaned epochs

I think we didn't add AR global for pre-ICA cleaning in the past because it would reject just too many epochs. I could remove that option

That said, I've used the existing AR global rejection after ICA successfully for a long time. So we should offer

ica_reject: None | dict[str, float] | Literal["autoreject_local"]
reject: None | dict[str, float] | Literal["autoreject_local", "autoreject_global"]

WDYT?

hoechenberger commented 10 months ago

@agramfort What's your feeling here? We didn't support AR global before ICA in the past. Maybe it should stay that way?

But AR local before ICA makes total sense, I'm getting really nice results and much better decompositions than when using fixed global thresholds with my data

hoechenberger commented 10 months ago

@larsoner I've removed support for global autoreject before ICA.

larsoner commented 10 months ago

Nice @hoechenberger !

hoechenberger commented 10 months ago

Thanks @larsoner