mne-bids-pipeline
mne-bids-pipeline copied to clipboard
mne-tools
ENH: Add mf_bads option
One solution that should work for this would be something like:
mf_bads: Literal["run", "union"] = "run"
where changing to "union" it will use the union of all bads across all data that will be maxwell filtered (runs + noise + rest). This actually in some sense is a safer option because the autobad only sometimes finds bad channels that it should find, so by combining across runs with a union is probably safest / most conservative.
Originally posted by @larsoner in https://github.com/mne-tools/mne-bids-pipeline/issues/759#issuecomment-1623806155
The more I think about this the less it seems to make sense to allow "run" -- I think we should use the union of bads across all runs to be safe in terms of rank etc. Especially in the case where one does bad channel detection/setting without maxwell filter things will only behave correctly with a "union"-type behavior -- otherwise you end up with different runs with different bads marked. At the end of the day we create one cov, one epo, one inv etc. and it will (I think) only reflect the bads selected for the first run.
Another way to think of it is that "runs" can really constitute a convenient way to split the data when saving into manageable chunks. In this sense the bads-per-run essentially create long, channel-specific BAD annotations. Given that we don't have a good way to do time-varying channel-specific interpolation safely (i.e., while accounting for rank properly etc.) it seems safest to just use the union as if it were a single long recording (and you've just used multiple segments of the data independently to find the bads).
Another option would be to concatenate the raws from all runs and come up with a single set of bads per subject rather than per run. But I'm not sure there are many advantages to this approach, and the asseciated memory consumption would be too great I think.
@hoechenberger okay to to treat it as a bugfix and use union of bads across runs rather than adding an option for it?
Makes sense to me, but I wouldn't label it a bug fix but rather a behavior change
