Skip to content
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

feat: Enable Jacobians only for PEPOLAR by default, allow forcing #3443

Merged
merged 1 commit 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
12 changes: 10 additions & 2 deletions fmriprep/cli/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@
action='store',
nargs='+',
default=[],
choices=['bbr', 'no-bbr', 'syn-sdc'],
choices=['bbr', 'no-bbr', 'syn-sdc', 'fmap-jacobian'],
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'
Expand Down Expand Up @@ -801,12 +801,20 @@
config.from_dict(vars(opts), init=['nipype'])

# Consistency checks
if 'bbr' in config.workflow.force and 'no-bbr' in config.workflow.force:
force_set = set(config.workflow.force)
ignore_set = set(config.workflow.ignore)
if {'bbr', 'no-bbr'} <= force_set:
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 'fmap-jacobian' in force_set & ignore_set:
msg = (

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

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/parser.py#L813

Added line #L813 was not covered by tests
'Cannot force and ignore fieldmap Jacobian correction. '
'Remove `fmap-jacobian` from either the `--force` or the `--ignore` option.'
)
raise ValueError(msg)

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

View check run for this annotation

Codecov / codecov/patch

fmriprep/cli/parser.py#L817

Added line #L817 was not covered by tests

if not config.execution.notrack:
import importlib.util
Expand Down
11 changes: 11 additions & 0 deletions fmriprep/workflows/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@
for field in ['fmap', 'fmap_ref', 'fmap_coeff', 'fmap_mask', 'fmap_id', 'sdc_method']
}

estimator_types = {est.bids_id: est.method for est in all_estimators}
fmap_estimators = []
if all_estimators:
# Find precomputed fieldmaps that apply to this workflow
Expand Down Expand Up @@ -738,6 +739,15 @@
for bold_series in bold_runs:
bold_file = bold_series[0]
fieldmap_id = estimator_map.get(bold_file)
jacobian = False

if fieldmap_id:
if 'fmap-jacobian' in config.workflow.force:
jacobian = True

Check warning on line 746 in fmriprep/workflows/base.py

View check run for this annotation

Codecov / codecov/patch

fmriprep/workflows/base.py#L746

Added line #L746 was not covered by tests
elif 'fmap-jacobian' not in config.workflow.ignore:
# Default behavior is to only use Jacobians for PEPOLAR fieldmaps
est_type = estimator_types[fieldmap_id]
jacobian = est_type == est_type.__class__.PEPOLAR

functional_cache = {}
if config.execution.derivatives:
Expand All @@ -758,6 +768,7 @@
bold_series=bold_series,
precomputed=functional_cache,
fieldmap_id=fieldmap_id,
jacobian=jacobian,
)
if bold_wf is None:
continue
Expand Down
9 changes: 6 additions & 3 deletions fmriprep/workflows/bold/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def init_bold_wf(
bold_series: list[str],
precomputed: dict = None,
fieldmap_id: str | None = None,
jacobian: bool = False,
) -> pe.Workflow:
"""
This workflow controls the functional preprocessing stages of *fMRIPrep*.
Expand Down Expand Up @@ -253,6 +254,7 @@ def init_bold_wf(
bold_series=bold_series,
precomputed=precomputed,
fieldmap_id=fieldmap_id,
jacobian=jacobian,
omp_nthreads=omp_nthreads,
)

Expand Down Expand Up @@ -292,6 +294,7 @@ def init_bold_wf(
bold_native_wf = init_bold_native_wf(
bold_series=bold_series,
fieldmap_id=fieldmap_id,
jacobian=jacobian,
omp_nthreads=omp_nthreads,
)

Expand Down Expand Up @@ -383,7 +386,7 @@ def init_bold_wf(
fieldmap_id=fieldmap_id if not multiecho else None,
omp_nthreads=omp_nthreads,
mem_gb=mem_gb,
jacobian='fmap-jacobian' not in config.workflow.ignore,
jacobian=jacobian,
name='bold_anat_wf',
)
bold_anat_wf.inputs.inputnode.resolution = 'native'
Expand Down Expand Up @@ -443,7 +446,7 @@ def init_bold_wf(
fieldmap_id=fieldmap_id if not multiecho else None,
omp_nthreads=omp_nthreads,
mem_gb=mem_gb,
jacobian='fmap-jacobian' not in config.workflow.ignore,
jacobian=jacobian,
name='bold_std_wf',
)
ds_bold_std_wf = init_ds_volumes_wf(
Expand Down Expand Up @@ -550,7 +553,7 @@ def init_bold_wf(
fieldmap_id=fieldmap_id if not multiecho else None,
omp_nthreads=omp_nthreads,
mem_gb=mem_gb,
jacobian='fmap-jacobian' not in config.workflow.ignore,
jacobian=jacobian,
name='bold_MNI6_wf',
)

Expand Down
4 changes: 3 additions & 1 deletion fmriprep/workflows/bold/fit.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ def init_bold_fit_wf(
bold_series: list[str],
precomputed: dict = None,
fieldmap_id: str | None = None,
jacobian: bool = False,
omp_nthreads: int = 1,
name: str = 'bold_fit_wf',
) -> pe.Workflow:
Expand Down Expand Up @@ -668,6 +669,7 @@ def init_bold_native_wf(
*,
bold_series: list[str],
fieldmap_id: str | None = None,
jacobian: bool = False,
omp_nthreads: int = 1,
name: str = 'bold_native_wf',
) -> pe.Workflow:
Expand Down Expand Up @@ -875,7 +877,7 @@ def init_bold_native_wf(

# Resample to boldref
boldref_bold = pe.Node(
ResampleSeries(jacobian='fmap-jacobian' not in config.workflow.ignore),
ResampleSeries(jacobian=jacobian),
name='boldref_bold',
n_procs=omp_nthreads,
mem_gb=mem_gb['resampled'],
Expand Down
Loading