Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions fmriprep/cli/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@
'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):
Expand Down Expand Up @@ -338,6 +341,19 @@
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='*',
Expand Down Expand Up @@ -392,17 +408,13 @@
)
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',
Expand Down Expand Up @@ -624,10 +636,9 @@
)
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
Expand Down Expand Up @@ -789,6 +800,14 @@
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 = (

Check warning on line 805 in fmriprep/cli/parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/parser.py#L805

Added line #L805 was not covered by tests
'Cannot force and disable boundary-based registration at the same time. '
'Remove `bbr` or `no-bbr` from the `--force` options.'
)
raise ValueError(msg)

Check warning on line 809 in fmriprep/cli/parser.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/parser.py#L809

Added line #L809 was not covered by tests

if not config.execution.notrack:
import importlib.util

Expand Down
2 changes: 2 additions & 0 deletions fmriprep/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion fmriprep/data/reports-spec-func.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion fmriprep/data/reports-spec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions fmriprep/data/tests/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand Down
6 changes: 3 additions & 3 deletions fmriprep/workflows/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
)

Expand Down Expand Up @@ -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,
Expand All @@ -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]

Expand Down
9 changes: 8 additions & 1 deletion fmriprep/workflows/bold/fit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down
34 changes: 16 additions & 18 deletions fmriprep/workflows/tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -135,9 +135,9 @@ def _make_params(
run_msmsulc,
skull_strip_t1w,
use_syn_sdc,
force_syn,
freesurfer,
ignore,
force,
bids_filters,
)

Expand All @@ -147,7 +147,6 @@ def _make_params(
@pytest.mark.parametrize(
(
'bold2anat_init',
'use_bbr',
'dummy_scans',
'me_output_echos',
'medial_surface_nan',
Expand All @@ -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),
Expand All @@ -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'}}),
],
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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()

Expand Down
Loading