feat(cli): Create --force flag that accepts a list, replacing individual --force-* flags (#3442)

This is in preparation to allow `--force fmap-jacobian` (see #3367). I
want to first establish the `--force` flag as a replacement for
`--force-bbr`, `--force-no-bbr` and `--force-syn`.

cc @mgxd @oesteban I'm not sure if we've talked about this before. I
think it's an overall worthwhile cleanup.

Author: effigies

fmriprep/cli/parser.py

@@ -48,6 +48,9 @@ def _build_parser(**kwargs):
         'aroma_err_on_warn': (None, '24.0.0'),
         'bold2t1w_init': ('--bold2anat-init', '24.2.0'),
         'bold2t1w_dof': ('--bold2anat-dof', '24.2.0'),
+        'force_bbr': ('--force bbr', '26.0.0'),
+        'force_no_bbr': ('--force no-bbr', '26.0.0'),
+        'force_syn': ('--force syn-sdc', '26.0.0'),
     }
 
     class DeprecatedAction(Action):
@@ -338,6 +341,19 @@ def _slice_time_ref(value, parser):
         help='Ignore selected aspects of the input dataset to disable corresponding '
         'parts of the workflow (a space delimited list)',
     )
+    g_conf.add_argument(
+        '--force',
+        required=False,
+        action='store',
+        nargs='+',
+        default=[],
+        choices=['bbr', 'no-bbr', 'syn-sdc'],
+        help='Force selected processing choices, overriding automatic selections '
+        '(a space delimited list).\n'
+        ' * [no-]bbr: Use/disable boundary-based registration for BOLD-to-T1w coregistration\n'
+        '             (No goodness-of-fit checks)\n'
+        ' * syn-sdc: Calculate SyN-SDC correction *in addition* to other fieldmaps\n',
+    )
     g_conf.add_argument(
         '--output-spaces',
         nargs='*',
@@ -392,17 +408,13 @@ def _slice_time_ref(value, parser):
     )
     g_conf.add_argument(
         '--force-bbr',
-        action='store_true',
-        dest='use_bbr',
-        default=None,
-        help='Always use boundary-based registration (no goodness-of-fit checks)',
+        action=DeprecatedAction,
+        help='Deprecated - use `--force bbr` instead.',
     )
     g_conf.add_argument(
         '--force-no-bbr',
-        action='store_false',
-        dest='use_bbr',
-        default=None,
-        help='Do not use boundary-based registration (no goodness-of-fit checks)',
+        action=DeprecatedAction,
+        help='Deprecated - use `--force no-bbr` instead.',
     )
     g_conf.add_argument(
         '--slice-time-ref',
@@ -624,10 +636,9 @@ def _slice_time_ref(value, parser):
     )
     g_syn.add_argument(
         '--force-syn',
-        action='store_true',
+        action=DeprecatedAction,
         default=False,
-        help='EXPERIMENTAL/TEMPORARY: Use SyN correction in addition to '
-        'fieldmap correction, if available',
+        help='Deprecated - use `--force syn-sdc` instead.',
     )
 
     # FreeSurfer options
@@ -789,6 +800,14 @@ def parse_args(args=None, namespace=None):
     config.execution.log_level = int(max(25 - 5 * opts.verbose_count, logging.DEBUG))
     config.from_dict(vars(opts), init=['nipype'])
 
+    # Consistency checks
+    if 'bbr' in config.workflow.force and 'no-bbr' in config.workflow.force:
+        msg = (
+            'Cannot force and disable boundary-based registration at the same time. '
+            'Remove `bbr` or `no-bbr` from the `--force` options.'
+        )
+        raise ValueError(msg)
+
     if not config.execution.notrack:
         import importlib.util
 

fmriprep/config.py

@@ -581,6 +581,8 @@ class workflow(_Config):
     """Adjust pipeline to reuse base template of existing longitudinal freesurfer"""
     ignore = None
     """Ignore particular steps for *fMRIPrep*."""
+    force = None
+    """Force particular steps for *fMRIPrep*."""
     level = None
     """Level of preprocessing to complete. One of ['minimal', 'resampling', 'full']."""
     longitudinal = False

fmriprep/data/reports-spec-func.yml

@@ -78,7 +78,7 @@ sections:
     static: false
     subtitle: Susceptibility distortion correction
   - bids: {datatype: figures, desc: forcedsyn, suffix: bold}
-    caption: The dataset contained some fieldmap information, but the argument <code>--force-syn</code>
+    caption: The dataset contained some fieldmap information, but the argument <code>--force syn-sdc</code>
       was used. The higher-priority SDC method was used. Here, we show the results
       of performing SyN-based SDC on the EPI for comparison.
     static: false

fmriprep/data/reports-spec.yml

@@ -104,7 +104,7 @@ sections:
     static: false
     subtitle: Susceptibility distortion correction
   - bids: {datatype: figures, desc: forcedsyn, suffix: bold}
-    caption: The dataset contained some fieldmap information, but the argument <code>--force-syn</code>
+    caption: The dataset contained some fieldmap information, but the argument <code>--force syn-sdc</code>
       was used. The higher-priority SDC method was used. Here, we show the results
       of performing SyN-based SDC on the EPI for comparison.
     static: false

fmriprep/data/tests/config.toml

@@ -34,6 +34,7 @@ aroma_err_on_warn = false
 aroma_melodic_dim = -200
 bold2anat_dof = 6
 fmap_bspline = false
+force = []
 force_syn = false
 hires = true
 ignore = []

fmriprep/workflows/base.py

@@ -557,7 +557,7 @@ def init_single_subject_wf(subject_id: str):
         bold_data=bold_runs,
         ignore_fieldmaps='fieldmaps' in config.workflow.ignore,
         use_syn=config.workflow.use_syn_sdc,
-        force_syn=config.workflow.force_syn,
+        force_syn='syn-sdc' in config.workflow.force,
         filters=config.execution.get().get('bids_filters', {}).get('fmap'),
     )
 
@@ -846,7 +846,7 @@ def map_fieldmap_estimation(
     # In the case where fieldmaps are ignored and `--use-syn-sdc` is requested,
     # SDCFlows `find_estimators` still receives a full layout (which includes the fmap modality)
     # and will not calculate fmapless schemes.
-    # Similarly, if fieldmaps are ignored and `--force-syn` is requested,
+    # Similarly, if fieldmaps are ignored and `--force syn-sdc` is requested,
     # `fmapless` should be set to True to ensure BOLD targets are found to be corrected.
     fmap_estimators = find_estimators(
         layout=layout,
@@ -870,7 +870,7 @@ def map_fieldmap_estimation(
     if ignore_fieldmaps and any(f.method == fm.EstimatorType.ANAT for f in fmap_estimators):
         config.loggers.workflow.info(
             'Option "--ignore fieldmaps" was set, but either "--use-syn-sdc" '
-            'or "--force-syn" were given, so fieldmap-less estimation will be executed.'
+            'or "--force syn-sdc" were given, so fieldmap-less estimation will be executed.'
         )
         fmap_estimators = [f for f in fmap_estimators if f.method == fm.EstimatorType.ANAT]
 

fmriprep/workflows/bold/fit.py

@@ -613,11 +613,18 @@ def init_bold_fit_wf(
         ])  # fmt:skip
 
     if not boldref2anat_xform:
+        use_bbr = (
+            True
+            if 'bbr' in config.workflow.force
+            else False
+            if 'no-bbr' in config.workflow.force
+            else None
+        )
         # calculate BOLD registration to T1w
         bold_reg_wf = init_bold_reg_wf(
             bold2anat_dof=config.workflow.bold2anat_dof,
             bold2anat_init=config.workflow.bold2anat_init,
-            use_bbr=config.workflow.use_bbr,
+            use_bbr=use_bbr,
             freesurfer=config.workflow.run_reconall,
             omp_nthreads=omp_nthreads,
             mem_gb=mem_gb['resampled'],

fmriprep/workflows/tests/test_base.py

@@ -106,7 +106,6 @@ def bids_root(tmp_path_factory):
 
 def _make_params(
     bold2anat_init: str = 'auto',
-    use_bbr: bool | None = None,
     dummy_scans: int | None = None,
     me_output_echos: bool = False,
     medial_surface_nan: bool = False,
@@ -115,18 +114,19 @@ def _make_params(
     run_msmsulc: bool = True,
     skull_strip_t1w: str = 'auto',
     use_syn_sdc: str | bool = False,
-    force_syn: bool = False,
     freesurfer: bool = True,
     ignore: list[str] = None,
+    force: list[str] = None,
     bids_filters: dict = None,
 ):
     if ignore is None:
         ignore = []
+    if force is None:
+        force = []
     if bids_filters is None:
         bids_filters = {}
     return (
         bold2anat_init,
-        use_bbr,
         dummy_scans,
         me_output_echos,
         medial_surface_nan,
@@ -135,9 +135,9 @@ def _make_params(
         run_msmsulc,
         skull_strip_t1w,
         use_syn_sdc,
-        force_syn,
         freesurfer,
         ignore,
+        force,
         bids_filters,
     )
 
@@ -147,7 +147,6 @@ def _make_params(
 @pytest.mark.parametrize(
     (
         'bold2anat_init',
-        'use_bbr',
         'dummy_scans',
         'me_output_echos',
         'medial_surface_nan',
@@ -156,21 +155,21 @@ def _make_params(
         'run_msmsulc',
         'skull_strip_t1w',
         'use_syn_sdc',
-        'force_syn',
         'freesurfer',
         'ignore',
+        'force',
         'bids_filters',
     ),
     [
         _make_params(),
         _make_params(bold2anat_init='t1w'),
         _make_params(bold2anat_init='t2w'),
         _make_params(bold2anat_init='header'),
-        _make_params(use_bbr=True),
-        _make_params(use_bbr=False),
-        _make_params(bold2anat_init='header', use_bbr=True),
+        _make_params(force=['bbr']),
+        _make_params(force=['no-bbr']),
+        _make_params(bold2anat_init='header', force=['bbr']),
         # Currently disabled
-        # _make_params(bold2anat_init="header", use_bbr=False),
+        # _make_params(bold2anat_init="header", force=['no-bbr']),
         _make_params(dummy_scans=2),
         _make_params(me_output_echos=True),
         _make_params(medial_surface_nan=True),
@@ -180,14 +179,14 @@ def _make_params(
         _make_params(cifti_output='91k', run_msmsulc=False),
         _make_params(skull_strip_t1w='force'),
         _make_params(skull_strip_t1w='skip'),
-        _make_params(use_syn_sdc='warn', force_syn=True, ignore=['fieldmaps']),
+        _make_params(use_syn_sdc='warn', ignore=['fieldmaps'], force=['syn-sdc']),
         _make_params(freesurfer=False),
-        _make_params(freesurfer=False, use_bbr=True),
-        _make_params(freesurfer=False, use_bbr=False),
+        _make_params(freesurfer=False, force=['bbr']),
+        _make_params(freesurfer=False, force=['no-bbr']),
         # Currently unsupported:
         # _make_params(freesurfer=False, bold2anat_init="header"),
-        # _make_params(freesurfer=False, bold2anat_init="header", use_bbr=True),
-        # _make_params(freesurfer=False, bold2anat_init="header", use_bbr=False),
+        # _make_params(freesurfer=False, bold2anat_init="header", force=['bbr']),
+        # _make_params(freesurfer=False, bold2anat_init="header", force=['no-bbr']),
         # Regression test for gh-3154:
         _make_params(bids_filters={'sbref': {'suffix': 'sbref'}}),
     ],
@@ -198,7 +197,6 @@ def test_init_fmriprep_wf(
     level: str,
     anat_only: bool,
     bold2anat_init: str,
-    use_bbr: bool | None,
     dummy_scans: int | None,
     me_output_echos: bool,
     medial_surface_nan: bool,
@@ -207,16 +205,15 @@ def test_init_fmriprep_wf(
     run_msmsulc: bool,
     skull_strip_t1w: str,
     use_syn_sdc: str | bool,
-    force_syn: bool,
     freesurfer: bool,
     ignore: list[str],
+    force: list[str],
     bids_filters: dict,
 ):
     with mock_config(bids_dir=bids_root):
         config.workflow.level = level
         config.workflow.anat_only = anat_only
         config.workflow.bold2anat_init = bold2anat_init
-        config.workflow.use_bbr = use_bbr
         config.workflow.dummy_scans = dummy_scans
         config.execution.me_output_echos = me_output_echos
         config.workflow.medial_surface_nan = medial_surface_nan
@@ -226,6 +223,7 @@ def test_init_fmriprep_wf(
         config.workflow.cifti_output = cifti_output
         config.workflow.run_reconall = freesurfer
         config.workflow.ignore = ignore
+        config.workflow.force = force
         with patch.dict('fmriprep.config.execution.bids_filters', bids_filters):
             wf = init_fmriprep_wf()