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
143 stars 68 forks source link

logging order in maxfilter step is confusing #985

Closed drammock closed 2 months ago

drammock commented 3 months ago

A colleague and I experienced a lot of head-scratching because of inaccurate (order/content) of log messages:

│08:01:48│ ⏳️ sub-215 ses-b Applying tSSS (6.0 sec, corr=0.95) with MC to experimental data
│08:01:50│ ⏳️ sub-215 ses-b Marking 3 channels as bad.
│08:01:50│ ⏳️ sub-215 ses-b Destination is 0.0 mm and 0.0° from the original head position
│08:03:15│ ⏳️ sub-215 ses-b Filtering cHPI
│08:03:26│ ⏳️ sub-215 ses-b Writing sub-215/ses-b/meg/sub-215_ses-b_task-AmplitudeModulatedTones_proc-sss_raw.fif
│08:03:30│ ⏳️ sub-215 ses-b Adding Maxwell filtered raw data to report.
│08:03:43│ ⏳️ sub-215 ses-b Saving report: /storage/badbaby-redux/bids-data/derivatives/mne-bids-pipeline/sub-215/ses-b/meg/sub-215_ses-b_task-AmplitudeModulatedTones_report.html

Looking at the code, the first message "Applying tSSS..." actually happens before the file is even loaded:

https://github.com/mne-tools/mne-bids-pipeline/blob/acc8e9823c9eb2171b3bdf1273511e442ea8b9d9/mne_bids_pipeline/steps/preprocessing/_03_maxfilter.py#L373-L381

But the order makes it seem like the bad channels are marked after tSSS was run. This might not have confused us if we'd seen the "Maxwell filtering" log message, but it never showed up:

https://github.com/mne-tools/mne-bids-pipeline/blob/acc8e9823c9eb2171b3bdf1273511e442ea8b9d9/mne_bids_pipeline/steps/preprocessing/_03_maxfilter.py#L428-L429

(caveat: log is from a parallel run with 2 subj & 2 sessions, and I've edited it to only show 1 subj/session. But ctrl+f searching through the log confirms that "Maxwell filtering" never shows up for any of the subjects/sessions.)

possible solutions:

  1. change the first message to "Preparing to apply tSSS..."
  2. fix the missing log message
  3. both 1 & 2

cc @mdclarke

larsoner commented 3 months ago

+1 for (3)