From f30c2fd749baba31bf16bdd3ce2ab66c98126b16 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Fri, 1 Mar 2024 14:10:48 -0500 Subject: [PATCH 1/7] FIX: Ensure templates are pulled on workflow construction --- smriprep/utils/misc.py | 27 +++++++++++++++++++++++++++ smriprep/workflows/anatomical.py | 3 +++ 2 files changed, 30 insertions(+) diff --git a/smriprep/utils/misc.py b/smriprep/utils/misc.py index 3c473b49a2..60e6f726fd 100644 --- a/smriprep/utils/misc.py +++ b/smriprep/utils/misc.py @@ -21,6 +21,11 @@ # https://www.nipreps.org/community/licensing/ # """Self-contained utilities to be used within Function nodes.""" +from pathlib import Path +import typing as ty + +if ty.TYPE_CHECKING: + from niworkflows.utils.spaces import Reference def apply_lut(in_dseg, lut, newpath=None): @@ -95,3 +100,25 @@ def fs_isRunning(subjects_dir, subject_id, mtime_tol=86400, logger=None): if logger: logger.warn(f'Removed "IsRunning*" files found under {subj_dir}') return subjects_dir + + +def get_template_t1w(template: str, sloppy: bool = False) -> Path: + """Query templateflow for the T1w to ensure it is present on the filesystem.""" + import templateflow.api as tf + + spec = {} + _space = template.split(':', 1) + if len(_space) > 1: + spec['cohort'] = _space[1].replace('cohort-', '') + space = _space[0] + + available_res = tf.TF_LAYOUT.get_resolutions(template=space) + if sloppy and 2 in available_res: + res = 2 + elif 1 in available_res: + res = 1 + else: + res = None + spec['resolution'] = res + + return tf.get(space, desc=None, suffix='T1w', **spec) diff --git a/smriprep/workflows/anatomical.py b/smriprep/workflows/anatomical.py index e65620da47..3b1534c960 100644 --- a/smriprep/workflows/anatomical.py +++ b/smriprep/workflows/anatomical.py @@ -973,6 +973,9 @@ def init_anat_fit_wf( templates = [] found_xfms = {} for template in spaces.get_spaces(nonstandard=False, dim=(3,)): + from ..utils.misc import get_template_t1w + + get_template_t1w(template, sloppy) xfms = precomputed.get('transforms', {}).get(template, {}) if set(xfms) != {'forward', 'reverse'}: templates.append(template) From 718843992f8fb1cc558dbfd5c6effa441dc58db4 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Fri, 1 Mar 2024 16:31:50 -0500 Subject: [PATCH 2/7] FIX: Ensure sloppy resolution is used --- smriprep/workflows/anatomical.py | 3 ++- smriprep/workflows/outputs.py | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/smriprep/workflows/anatomical.py b/smriprep/workflows/anatomical.py index 3b1534c960..f8b121b38b 100644 --- a/smriprep/workflows/anatomical.py +++ b/smriprep/workflows/anatomical.py @@ -277,7 +277,7 @@ def init_anat_preproc_wf( omp_nthreads=omp_nthreads, skull_strip_fixed_seed=skull_strip_fixed_seed, ) - template_iterator_wf = init_template_iterator_wf(spaces=spaces) + template_iterator_wf = init_template_iterator_wf(spaces=spaces, sloppy=sloppy) ds_std_volumes_wf = init_ds_anat_volumes_wf( bids_root=bids_root, output_dir=output_dir, @@ -725,6 +725,7 @@ def init_anat_fit_wf( spaces=spaces, freesurfer=freesurfer, output_dir=output_dir, + sloppy=sloppy, ) # fmt:off workflow.connect([ diff --git a/smriprep/workflows/outputs.py b/smriprep/workflows/outputs.py index 82f0f668c8..0ac3fba8df 100644 --- a/smriprep/workflows/outputs.py +++ b/smriprep/workflows/outputs.py @@ -35,10 +35,13 @@ from ..interfaces import DerivativesDataSink from ..interfaces.templateflow import TemplateFlowSelect +if ty.TYPE_CHECKING: + from niworkflows.utils.spaces import SpatialReferences + BIDS_TISSUE_ORDER = ('GM', 'WM', 'CSF') -def init_anat_reports_wf(*, spaces, freesurfer, output_dir, name='anat_reports_wf'): +def init_anat_reports_wf(*, spaces, freesurfer, output_dir, sloppy=False, name='anat_reports_wf'): """ Set up a battery of datasinks to store reports in the right location. @@ -131,7 +134,7 @@ def init_anat_reports_wf(*, spaces, freesurfer, output_dir, name='anat_reports_w # fmt:on if spaces._cached is not None and spaces.cached.references: - template_iterator_wf = init_template_iterator_wf(spaces=spaces) + template_iterator_wf = init_template_iterator_wf(spaces=spaces, sloppy=sloppy) t1w_std = pe.Node( ApplyTransforms( dimension=3, @@ -1112,7 +1115,12 @@ def init_anat_second_derivatives_wf( return workflow -def init_template_iterator_wf(*, spaces, name='template_iterator_wf'): +def init_template_iterator_wf( + *, + spaces: 'SpatialReferences', + sloppy: bool = False, + name='template_iterator_wf' +): """Prepare the necessary components to resample an image to a template space This produces a workflow with an unjoined iterable named "spacesource". @@ -1160,7 +1168,7 @@ def init_template_iterator_wf(*, spaces, name='template_iterator_wf'): run_without_submitting=True, ) select_tpl = pe.Node( - TemplateFlowSelect(resolution=1), name='select_tpl', run_without_submitting=True + TemplateFlowSelect(resolution=2 if sloppy else 1), name='select_tpl', run_without_submitting=True ) # fmt:off From b7573676358de1af34ff758051cd74ebfc7d7cb5 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 6 Mar 2024 11:49:11 -0500 Subject: [PATCH 3/7] FIX: Determine resolution by sloppy parameter --- smriprep/workflows/outputs.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/smriprep/workflows/outputs.py b/smriprep/workflows/outputs.py index 0ac3fba8df..b99609a71f 100644 --- a/smriprep/workflows/outputs.py +++ b/smriprep/workflows/outputs.py @@ -1168,7 +1168,7 @@ def init_template_iterator_wf( run_without_submitting=True, ) select_tpl = pe.Node( - TemplateFlowSelect(resolution=2 if sloppy else 1), name='select_tpl', run_without_submitting=True + TemplateFlowSelect(), name='select_tpl', run_without_submitting=True ) # fmt:off @@ -1185,7 +1185,7 @@ def init_template_iterator_wf( (spacesource, select_tpl, [ ('space', 'template'), ('cohort', 'cohort'), - (('resolution', _no_native), 'resolution'), + (('resolution', _no_native, sloppy), 'resolution'), ]), (spacesource, outputnode, [ ('space', 'space'), @@ -1251,10 +1251,6 @@ def _pick_cohort(in_template): return [_pick_cohort(v) for v in in_template] -def _fmt(in_template): - return in_template.replace(':', '_') - - def _empty_report(in_file=None): from pathlib import Path @@ -1276,11 +1272,11 @@ def _is_native(value): return value == 'native' -def _no_native(value): +def _no_native(value, sloppy=False): try: return int(value) except (TypeError, ValueError): - return 1 + return 2 if sloppy else 1 def _drop_path(in_path): From 97f4556ecb77cc8bd293c1bfbdbdfe52441effda Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 6 Mar 2024 13:50:53 -0500 Subject: [PATCH 4/7] RF: Strip out template fetching to separate function --- smriprep/interfaces/templateflow.py | 67 +++++++++++++++++------------ smriprep/utils/misc.py | 28 ------------ 2 files changed, 39 insertions(+), 56 deletions(-) diff --git a/smriprep/interfaces/templateflow.py b/smriprep/interfaces/templateflow.py index e7199c8b94..2a95893d48 100644 --- a/smriprep/interfaces/templateflow.py +++ b/smriprep/interfaces/templateflow.py @@ -108,34 +108,9 @@ def _run_interface(self, runtime): if isdefined(self.inputs.cohort): specs['cohort'] = self.inputs.cohort - name = self.inputs.template.strip(':').split(':', 1) - if len(name) > 1: - specs.update( - { - k: v - for modifier in name[1].split(':') - for k, v in [tuple(modifier.split('-'))] - if k not in specs - } - ) - - if specs['resolution'] and not isinstance(specs['resolution'], list): - specs['resolution'] = [specs['resolution']] - - available_resolutions = tf.TF_LAYOUT.get_resolutions(template=name[0]) - if specs['resolution'] and not set(specs['resolution']) & set(available_resolutions): - fallback_res = available_resolutions[0] if available_resolutions else None - LOGGER.warning( - f"Template {name[0]} does not have resolution(s): {specs['resolution']}." - f"Falling back to resolution: {fallback_res}." - ) - specs['resolution'] = fallback_res - - self._results['t1w_file'] = tf.get(name[0], desc=None, suffix='T1w', **specs) - - self._results['brain_mask'] = tf.get( - name[0], desc='brain', suffix='mask', **specs - ) or tf.get(name[0], label='brain', suffix='mask', **specs) + files = fetch_template_files(self.inputs.template, specs) + self._results['t1w_file'] = files['t1w'] + self._results['brain_mask'] = files['mask'] return runtime @@ -186,3 +161,39 @@ def _run_interface(self, runtime): descsplit = desc.split('-') self._results['spec'][descsplit[0]] = descsplit[1] return runtime + + +def fetch_template_files(template: str, specs: dict | None = None) -> dict: + if specs is None: + specs = {} + + name = template.strip(':').split(':', 1) + if len(name) > 1: + specs.update( + { + k: v + for modifier in name[1].split(':') + for k, v in [tuple(modifier.split('-'))] + if k not in specs + } + ) + + if specs['resolution'] and not isinstance(specs['resolution'], list): + specs['resolution'] = [specs['resolution']] + + available_resolutions = tf.TF_LAYOUT.get_resolutions(template=name[0]) + if specs['resolution'] and not set(specs['resolution']) & set(available_resolutions): + fallback_res = available_resolutions[0] if available_resolutions else None + LOGGER.warning( + f"Template {name[0]} does not have resolution(s): {specs['resolution']}." + f"Falling back to resolution: {fallback_res}." + ) + specs['resolution'] = fallback_res + + files = {} + files['t1w'] = tf.get(name[0], desc=None, suffix='T1w', **specs) + files['mask'] = ( + tf.get(name[0], desc='brain', suffix='mask', **specs) + or tf.get(name[0], label='brain', suffix='mask', **specs) + ) + return files diff --git a/smriprep/utils/misc.py b/smriprep/utils/misc.py index 60e6f726fd..2e399a7ad4 100644 --- a/smriprep/utils/misc.py +++ b/smriprep/utils/misc.py @@ -21,12 +21,6 @@ # https://www.nipreps.org/community/licensing/ # """Self-contained utilities to be used within Function nodes.""" -from pathlib import Path -import typing as ty - -if ty.TYPE_CHECKING: - from niworkflows.utils.spaces import Reference - def apply_lut(in_dseg, lut, newpath=None): """Map the input discrete segmentation to a new label set (lookup table, LUT).""" @@ -100,25 +94,3 @@ def fs_isRunning(subjects_dir, subject_id, mtime_tol=86400, logger=None): if logger: logger.warn(f'Removed "IsRunning*" files found under {subj_dir}') return subjects_dir - - -def get_template_t1w(template: str, sloppy: bool = False) -> Path: - """Query templateflow for the T1w to ensure it is present on the filesystem.""" - import templateflow.api as tf - - spec = {} - _space = template.split(':', 1) - if len(_space) > 1: - spec['cohort'] = _space[1].replace('cohort-', '') - space = _space[0] - - available_res = tf.TF_LAYOUT.get_resolutions(template=space) - if sloppy and 2 in available_res: - res = 2 - elif 1 in available_res: - res = 1 - else: - res = None - spec['resolution'] = res - - return tf.get(space, desc=None, suffix='T1w', **spec) From 5166979513efdc3a54a3255bd81abb62a36a1587 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Wed, 6 Mar 2024 14:25:54 -0500 Subject: [PATCH 5/7] FIX: Fetch templates during workflow construction --- smriprep/interfaces/templateflow.py | 17 ++++++++++++++--- smriprep/workflows/anatomical.py | 4 ++-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/smriprep/interfaces/templateflow.py b/smriprep/interfaces/templateflow.py index 2a95893d48..af38fa1b87 100644 --- a/smriprep/interfaces/templateflow.py +++ b/smriprep/interfaces/templateflow.py @@ -163,7 +163,11 @@ def _run_interface(self, runtime): return runtime -def fetch_template_files(template: str, specs: dict | None = None) -> dict: +def fetch_template_files( + template: str, + specs: dict | None = None, + sloppy: bool = False, +) -> dict: if specs is None: specs = {} @@ -178,11 +182,18 @@ def fetch_template_files(template: str, specs: dict | None = None) -> dict: } ) - if specs['resolution'] and not isinstance(specs['resolution'], list): + if res := specs.pop('res', None): + if res != 'native': + specs['resolution'] = res + + if not specs.get('resolution'): + specs['resolution'] = 2 if sloppy else 1 + + if specs.get('resolution') and not isinstance(specs['resolution'], list): specs['resolution'] = [specs['resolution']] available_resolutions = tf.TF_LAYOUT.get_resolutions(template=name[0]) - if specs['resolution'] and not set(specs['resolution']) & set(available_resolutions): + if specs.get('resolution') and not set(specs['resolution']) & set(available_resolutions): fallback_res = available_resolutions[0] if available_resolutions else None LOGGER.warning( f"Template {name[0]} does not have resolution(s): {specs['resolution']}." diff --git a/smriprep/workflows/anatomical.py b/smriprep/workflows/anatomical.py index f8b121b38b..4135923aae 100644 --- a/smriprep/workflows/anatomical.py +++ b/smriprep/workflows/anatomical.py @@ -974,9 +974,9 @@ def init_anat_fit_wf( templates = [] found_xfms = {} for template in spaces.get_spaces(nonstandard=False, dim=(3,)): - from ..utils.misc import get_template_t1w + from smriprep.interfaces.templateflow import fetch_template_files - get_template_t1w(template, sloppy) + fetch_template_files(template, specs=None, sloppy=sloppy) xfms = precomputed.get('transforms', {}).get(template, {}) if set(xfms) != {'forward', 'reverse'}: templates.append(template) From e351f9a4bdeca0744b396e6fef2770e37175cb46 Mon Sep 17 00:00:00 2001 From: mathiasg Date: Thu, 7 Mar 2024 11:48:00 -0500 Subject: [PATCH 6/7] RF: Move template fetching to iterator workflow, adhere to style --- smriprep/interfaces/templateflow.py | 5 ++--- smriprep/utils/misc.py | 1 + smriprep/workflows/anatomical.py | 3 --- smriprep/workflows/outputs.py | 14 +++++++------- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/smriprep/interfaces/templateflow.py b/smriprep/interfaces/templateflow.py index af38fa1b87..6a466d7d09 100644 --- a/smriprep/interfaces/templateflow.py +++ b/smriprep/interfaces/templateflow.py @@ -203,8 +203,7 @@ def fetch_template_files( files = {} files['t1w'] = tf.get(name[0], desc=None, suffix='T1w', **specs) - files['mask'] = ( - tf.get(name[0], desc='brain', suffix='mask', **specs) - or tf.get(name[0], label='brain', suffix='mask', **specs) + files['mask'] = tf.get(name[0], desc='brain', suffix='mask', **specs) or tf.get( + name[0], label='brain', suffix='mask', **specs ) return files diff --git a/smriprep/utils/misc.py b/smriprep/utils/misc.py index 2e399a7ad4..3c473b49a2 100644 --- a/smriprep/utils/misc.py +++ b/smriprep/utils/misc.py @@ -22,6 +22,7 @@ # """Self-contained utilities to be used within Function nodes.""" + def apply_lut(in_dseg, lut, newpath=None): """Map the input discrete segmentation to a new label set (lookup table, LUT).""" import nibabel as nb diff --git a/smriprep/workflows/anatomical.py b/smriprep/workflows/anatomical.py index 4135923aae..4bdfd285c5 100644 --- a/smriprep/workflows/anatomical.py +++ b/smriprep/workflows/anatomical.py @@ -974,9 +974,6 @@ def init_anat_fit_wf( templates = [] found_xfms = {} for template in spaces.get_spaces(nonstandard=False, dim=(3,)): - from smriprep.interfaces.templateflow import fetch_template_files - - fetch_template_files(template, specs=None, sloppy=sloppy) xfms = precomputed.get('transforms', {}).get(template, {}) if set(xfms) != {'forward', 'reverse'}: templates.append(template) diff --git a/smriprep/workflows/outputs.py b/smriprep/workflows/outputs.py index b99609a71f..9cf2f45ea3 100644 --- a/smriprep/workflows/outputs.py +++ b/smriprep/workflows/outputs.py @@ -1116,10 +1116,7 @@ def init_anat_second_derivatives_wf( def init_template_iterator_wf( - *, - spaces: 'SpatialReferences', - sloppy: bool = False, - name='template_iterator_wf' + *, spaces: 'SpatialReferences', sloppy: bool = False, name='template_iterator_wf' ): """Prepare the necessary components to resample an image to a template space @@ -1130,6 +1127,11 @@ def init_template_iterator_wf( The fields in `outputnode` can be used as if they come from a single template. """ + for template in spaces.get_spaces(nonstandard=False, dim=(3,)): + from smriprep.interfaces.templateflow import fetch_template_files + + fetch_template_files(template, specs=None, sloppy=sloppy) + workflow = pe.Workflow(name=name) inputnode = pe.Node( @@ -1167,9 +1169,7 @@ def init_template_iterator_wf( name='select_xfm', run_without_submitting=True, ) - select_tpl = pe.Node( - TemplateFlowSelect(), name='select_tpl', run_without_submitting=True - ) + select_tpl = pe.Node(TemplateFlowSelect(), name='select_tpl', run_without_submitting=True) # fmt:off workflow.connect([ From 9f13a869266c84afa2f3925eded69a94408625c9 Mon Sep 17 00:00:00 2001 From: Mathias Goncalves Date: Thu, 7 Mar 2024 14:45:29 -0500 Subject: [PATCH 7/7] Apply suggestions from code review Co-authored-by: Chris Markiewicz --- smriprep/workflows/outputs.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/smriprep/workflows/outputs.py b/smriprep/workflows/outputs.py index 9cf2f45ea3..dee84d17a0 100644 --- a/smriprep/workflows/outputs.py +++ b/smriprep/workflows/outputs.py @@ -33,7 +33,7 @@ from niworkflows.interfaces.utility import KeySelect from ..interfaces import DerivativesDataSink -from ..interfaces.templateflow import TemplateFlowSelect +from ..interfaces.templateflow import TemplateFlowSelect, fetch_template_files if ty.TYPE_CHECKING: from niworkflows.utils.spaces import SpatialReferences @@ -1128,8 +1128,6 @@ def init_template_iterator_wf( The fields in `outputnode` can be used as if they come from a single template. """ for template in spaces.get_spaces(nonstandard=False, dim=(3,)): - from smriprep.interfaces.templateflow import fetch_template_files - fetch_template_files(template, specs=None, sloppy=sloppy) workflow = pe.Workflow(name=name)