Merge pull request #399 from mgxd/fix/mcribs-no-segmentation

FIX: MCRIBS auto surface reconstruction logic

Author: mgxd

nibabies/cli/parser.py

@@ -28,6 +28,8 @@ def _build_parser():
         # parser attribute name: (replacement flag, version slated to be removed in)
         'bold2t1w_init': ('--bold2anat-init', '24.2.0'),
         'bold2t1w_dof': ('--bold2anat-dof', '24.2.0'),
+        'force_reconall': ('--surface-recon-method freesurfer', '24.2.0'),
+        'fs_no_reconall': ('--surface-recon-method none', '24.2.0'),
     }
 
     class DeprecatedAction(Action):
@@ -148,6 +150,11 @@ def _slice_time_ref(value, parser):
             raise parser.error(f'Slice time reference must be in range 0-1. Received {value}.')
         return value
 
+    def _str_none(val):
+        if not isinstance(val, str):
+            return val
+        return None if val.lower() == 'none' else val
+
     verstr = f'NiBabies v{config.environment.version}'
     currentv = Version(config.environment.version)
 
@@ -619,9 +626,9 @@ def _slice_time_ref(value, parser):
     )
     g_surfs_xor.add_argument(
         '--fs-no-reconall',
-        action='store_false',
+        action=DeprecatedAction,
         dest='run_reconall',
-        help='disable FreeSurfer surface preprocessing.',
+        help='Deprecated - use `--surface-recon-method none` instead.',
     )
 
     g_other = parser.add_argument_group('Other options')
@@ -750,14 +757,15 @@ def _slice_time_ref(value, parser):
     g_baby.add_argument(
         '--force-reconall',
         default=False,
-        action='store_true',
-        help='Force traditional FreeSurfer surface reconstruction.',
+        action=DeprecatedAction,
+        help='Deprecated - use `--surface-recon-method freesurfer` instead.',
     )
     g_baby.add_argument(
         '--surface-recon-method',
-        choices=('infantfs', 'freesurfer', 'mcribs', 'auto'),
+        choices=('auto', 'infantfs', 'freesurfer', 'mcribs', None),
+        type=_str_none,
         default='auto',
-        help='Method to use for surface reconstruction',
+        help='Method to use for surface reconstruction.',
     )
     g_baby.add_argument(
         '--reference-anat',

nibabies/cli/tests/test_parser.py

@@ -270,3 +270,34 @@ def test_derivatives(tmp_path):
         parser.parse_args(temp_args)
 
     _reset_config()
+
+
+@pytest.mark.parametrize(
+    ('args', 'expectation'),
+    [
+        ([], 'auto'),
+        (['--surface-recon-method', 'auto'], 'auto'),
+        (['--surface-recon-method', 'mcribs'], 'mcribs'),
+        (['--surface-recon-method', 'infantfs'], 'infantfs'),
+        (['--surface-recon-method', 'freesurfer'], 'freesurfer'),
+        (['--surface-recon-method', 'none'], None),
+        (['--surface-recon-method', 'None'], None),
+        (['--surface-recon-method', 123], (TypeError,)),
+    ],
+)
+def test_surface_recon_method(tmp_path, args, expectation):
+    """Check the correct parsing of the memory argument."""
+    datapath = tmp_path / 'data'
+    datapath.mkdir(exist_ok=True)
+    _fs_file = tmp_path / 'license.txt'
+    _fs_file.write_text('')
+
+    args = [str(datapath)] + MIN_ARGS[1:] + ['--fs-license-file', str(_fs_file)] + args
+
+    cm = nullcontext()
+    if isinstance(expectation, tuple):
+        cm = pytest.raises(expectation)
+
+    with cm:
+        opts = _build_parser().parse_args(args)
+        assert opts.surface_recon_method == expectation

nibabies/workflows/base.py

@@ -71,6 +71,9 @@
     from niworkflows.utils.spaces import SpatialReferences
 
 
+AUTO_T2W_MAX_AGE = 8
+
+
 def init_nibabies_wf(subworkflows_list):
     """
     Build *NiBabies*'s pipeline.
@@ -105,8 +108,8 @@ def init_nibabies_wf(subworkflows_list):
     nibabies_wf.base_dir = config.execution.work_dir
 
     execution_spaces = init_execution_spaces()
-    freesurfer = config.workflow.surface_recon_method is not None
-    if freesurfer:
+    surface_recon = config.workflow.surface_recon_method is not None
+    if surface_recon:
         fsdir = pe.Node(
             BIDSFreeSurferDir(
                 derivatives=config.execution.output_dir,
@@ -154,7 +157,7 @@ def init_nibabies_wf(subworkflows_list):
         single_subject_wf.config['execution']['crashdump_dir'] = str(log_dir)
         for node in single_subject_wf._get_all_nodes():
             node.config = deepcopy(single_subject_wf.config)
-        if freesurfer:
+        if surface_recon:
             nibabies_wf.connect(fsdir, 'subjects_dir', single_subject_wf, 'inputnode.subjects_dir')
         else:
             nibabies_wf.add_nodes([single_subject_wf])
@@ -327,30 +330,32 @@ def init_single_subject_wf(
     # Determine some session level options here, as we should have
     # all the required information
     if recon_method == 'auto':
-        if age <= 8:
+        if age <= AUTO_T2W_MAX_AGE and anatomical_cache.get('t2w_aseg'):
+            # do not force mcribs without a vetted segmentation
             recon_method = 'mcribs'
         elif age <= 24:
             recon_method = 'infantfs'
         else:
             recon_method = 'freesurfer'
 
-    preferred_anat = config.execution.reference_anat
+    requested_anat = config.execution.reference_anat
     t1w = subject_data['t1w']
     t2w = subject_data['t2w']
     single_anat = False
-    if not t1w and t2w:
+
+    if not (t1w and t2w):
         single_anat = True
         reference_anat = 'T1w' if t1w else 'T2w'
-        if preferred_anat and reference_anat != preferred_anat:
+        if requested_anat and reference_anat != requested_anat:
             raise AttributeError(
-                f'Requested to use {preferred_anat} as anatomical reference but none available'
+                f'Requested to use {requested_anat} as anatomical reference but none available'
             )
-    else:
-        if not (reference_anat := preferred_anat):
-            if recon_method is None:
-                reference_anat = 'T2w' if age <= 8 else 'T1w'
-            else:
-                reference_anat = 'T2w' if recon_method == 'mcribs' else 'T1w'
+    elif (reference_anat := requested_anat) is None:  # Both available with no preference
+        reference_anat = (
+            'T2w'
+            if any((recon_method == 'none' and age <= AUTO_T2W_MAX_AGE, recon_method == 'mcribs'))
+            else 'T1w'
+        )
 
     anat = reference_anat.lower()  # To be used for workflow connections