Merge pull request #1274 from romainVala/master

FIX: Remove long paths from all LTA output files

Author: effigies

.zenodo.json

@@ -83,6 +83,11 @@
       "affiliation": "Department of Psychology, Stanford University",
       "orcid": "0000-0001-6755-0259"
     },
+    {
+      "name": "Valabregue, Romain",
+      "affiliation": "Cenir, ICM, Paris",
+      "orcid": "0000-0002-1814-9570"
+    },
     {
       "name": "Gorgolewski, Krzysztof J.",
       "affiliation": "Department of Psychology, Stanford University",

fmriprep/interfaces/freesurfer.py

@@ -34,7 +34,9 @@
 )
 from nipype.interfaces import freesurfer as fs
 from nipype.interfaces.base import SimpleInterface
-from nipype.interfaces.freesurfer.preprocess import ConcatenateLTA
+from nipype.interfaces.freesurfer.preprocess import ConcatenateLTA, RobustRegister
+from nipype.interfaces.freesurfer.utils import LTAConvert
+from niworkflows.interfaces.registration import BBRegisterRPT, MRICoregRPT
 
 
 class StructuralReference(fs.RobustTemplate):
@@ -190,7 +192,54 @@ def _run_interface(self, runtime):
         return runtime
 
 
-class PatchedConcatenateLTA(ConcatenateLTA):
+class TruncateLTA(object):
+    """Mixin to ensure that LTA files do not store overly long paths,
+    which lead to segmentation faults when read by FreeSurfer tools.
+
+    See the following issues for discussion:
+
+    * https://github.com/freesurfer/freesurfer/pull/180
+    * https://github.com/poldracklab/fmriprep/issues/768
+    * https://github.com/poldracklab/fmriprep/pull/778
+    * https://github.com/poldracklab/fmriprep/issues/1268
+    * https://github.com/poldracklab/fmriprep/pull/1274
+    """
+
+    # Use a tuple in case some object produces multiple transforms
+    lta_outputs = ('out_lta_file',)
+
+    def _post_run_hook(self, runtime):
+
+        outputs = self._list_outputs()
+
+        for lta_name in self.lta_outputs:
+            lta_file = outputs[lta_name]
+            if not isdefined(lta_file):
+                continue
+
+            with open(lta_file, 'r') as f:
+                lines = f.readlines()
+
+            fixed = False
+            newfile = []
+
+            for line in lines:
+                if line.startswith('filename = ') and len(line.strip("\n")) >= 255:
+                    fixed = True
+                    newfile.append('filename = path_too_long\n')
+                else:
+                    newfile.append(line)
+
+            if fixed:
+                with open(lta_file, 'w') as f:
+                    f.write(''.join(newfile))
+
+        runtime = super(TruncateLTA, self)._post_run_hook(runtime)
+
+        return runtime
+
+
+class PatchedConcatenateLTA(TruncateLTA, ConcatenateLTA):
     """
     A temporarily patched version of ``fs.ConcatenateLTA`` to recover from
     `this bug <https://www.mail-archive.com/[email protected]/msg55520.html>`_
@@ -199,27 +248,30 @@ class PatchedConcatenateLTA(ConcatenateLTA):
 
     The original FMRIPREP's issue is found
     `here <https://github.com/poldracklab/fmriprep/issues/768>`__.
+
+    the fix is now done through mixin with TruncateLTA
     """
+    lta_outputs = ['out_file']
 
-    def _list_outputs(self):
-        outputs = super(PatchedConcatenateLTA, self)._list_outputs()
-
-        with open(outputs['out_file'], 'r') as f:
-            lines = f.readlines()
-
-        fixed = False
-        newfile = []
-        for line in lines:
-            if line.startswith('filename = ') and len(line.strip("\n")) >= 255:
-                fixed = True
-                newfile.append('filename = path_too_long\n')
-            else:
-                newfile.append(line)
-
-        if fixed:
-            with open(outputs['out_file'], 'w') as f:
-                f.write(''.join(newfile))
-        return outputs
+
+class PatchedLTAConvert(TruncateLTA, LTAConvert):
+    """
+    LTAconvert is producing a lta file refer as out_lta
+    truncate filename through mixin TruncateLTA
+    """
+    lta_outputs = ('out_lta',)
+
+
+class PatchedBBRegisterRPT(TruncateLTA, BBRegisterRPT):
+    pass
+
+
+class PatchedMRICoregRPT(TruncateLTA, MRICoregRPT):
+    pass
+
+
+class PatchedRobustRegister(TruncateLTA, RobustRegister):
+    lta_outputs = ('out_reg_file', 'half_source_xfm', 'half_targ_xfm')
 
 
 class RefineBrainMaskInputSpec(BaseInterfaceInputSpec):

fmriprep/workflows/anatomical.py

@@ -49,6 +49,9 @@
     ConcatAffines, RefineBrainMask,
 )
 from ..utils.misc import fix_multi_T1w_source_name, add_suffix
+from ..interfaces.freesurfer import (
+        PatchedLTAConvert as LTAConvert,
+        PatchedRobustRegister as RobustRegister)
 
 TEMPLATE_MAP = {
     'MNI152NLin2009cAsym': 'mni_icbm152_nlin_asym_09c',
@@ -533,7 +536,7 @@ def _get_first(in_list):
     # 2. Reorient template to RAS, if needed (mri_robust_template may set to LIA)
     t1_reorient = pe.Node(image.Reorient(), name='t1_reorient')
 
-    lta_to_fsl = pe.MapNode(fs.utils.LTAConvert(out_fsl=True), iterfield=['in_lta'],
+    lta_to_fsl = pe.MapNode(LTAConvert(out_fsl=True), iterfield=['in_lta'],
                             name='lta_to_fsl')
 
     concat_affines = pe.MapNode(
@@ -813,9 +816,9 @@ def init_surface_recon_wf(omp_nthreads, hires, name='surface_recon_wf'):
 
     skull_strip_extern = pe.Node(FSInjectBrainExtracted(), name='skull_strip_extern')
 
-    fsnative_2_t1_xfm = pe.Node(fs.RobustRegister(auto_sens=True, est_int_scale=True),
+    fsnative_2_t1_xfm = pe.Node(RobustRegister(auto_sens=True, est_int_scale=True),
                                 name='fsnative_2_t1_xfm')
-    t1_2_fsnative_xfm = pe.Node(fs.utils.LTAConvert(out_lta=True, invert=True),
+    t1_2_fsnative_xfm = pe.Node(LTAConvert(out_lta=True, invert=True),
                                 name='t1_2_fsnative_xfm')
 
     autorecon_resume_wf = init_autorecon_resume_wf(omp_nthreads=omp_nthreads)
@@ -1279,7 +1282,7 @@ def init_anat_derivatives_wf(output_dir, output_spaces, template, freesurfer,
         DerivativesDataSink(base_directory=output_dir, suffix=suffix_fmt(template, 'warp')),
         name='ds_t1_mni_warp', run_without_submitting=True)
 
-    lta_2_itk = pe.Node(fs.utils.LTAConvert(out_itk=True), name='lta_2_itk')
+    lta_2_itk = pe.Node(LTAConvert(out_itk=True), name='lta_2_itk')
 
     ds_t1_fsnative = pe.Node(
         DerivativesDataSink(base_directory=output_dir, suffix=suffix_fmt('fsnative', 'affine')),

fmriprep/workflows/bold/registration.py

@@ -17,8 +17,8 @@
 import os.path as op
 
 from nipype.pipeline import engine as pe
-from nipype.interfaces import utility as niu, fsl, c3, freesurfer as fs
-from niworkflows.interfaces.registration import FLIRTRPT, BBRegisterRPT, MRICoregRPT
+from nipype.interfaces import utility as niu, fsl, c3
+from niworkflows.interfaces.registration import FLIRTRPT
 from niworkflows.interfaces.utils import GenerateSamplingReference
 from niworkflows.interfaces.fixes import FixHeaderApplyTransforms as ApplyTransforms
 
@@ -28,7 +28,11 @@
 from ...interfaces.nilearn import Merge
 from ...interfaces.images import extract_wm
 # See https://github.com/poldracklab/fmriprep/issues/768
-from ...interfaces.freesurfer import PatchedConcatenateLTA as ConcatenateLTA
+from ...interfaces.freesurfer import (
+        PatchedConcatenateLTA as ConcatenateLTA,
+        PatchedBBRegisterRPT as BBRegisterRPT,
+        PatchedMRICoregRPT as MRICoregRPT,
+        PatchedLTAConvert as LTAConvert)
 
 
 DEFAULT_MEMORY_MIN_GB = 0.01
@@ -420,8 +424,8 @@ def init_bbreg_wf(use_bbr, bold2t1w_dof, omp_nthreads, name='bbreg_wf'):
     lta_concat = pe.Node(ConcatenateLTA(out_file='out.lta'), name='lta_concat')
     # XXX LTA-FSL-ITK may ultimately be able to be replaced with a straightforward
     # LTA-ITK transform, but right now the translation parameters are off.
-    lta2fsl_fwd = pe.Node(fs.utils.LTAConvert(out_fsl=True), name='lta2fsl_fwd')
-    lta2fsl_inv = pe.Node(fs.utils.LTAConvert(out_fsl=True, invert=True), name='lta2fsl_inv')
+    lta2fsl_fwd = pe.Node(LTAConvert(out_fsl=True), name='lta2fsl_fwd')
+    lta2fsl_inv = pe.Node(LTAConvert(out_fsl=True, invert=True), name='lta2fsl_inv')
     fsl2itk_fwd = pe.Node(c3.C3dAffineTool(fsl2ras=True, itk_transform=True),
                           name='fsl2itk_fwd', mem_gb=DEFAULT_MEMORY_MIN_GB)
     fsl2itk_inv = pe.Node(c3.C3dAffineTool(fsl2ras=True, itk_transform=True),
@@ -478,7 +482,7 @@ def init_bbreg_wf(use_bbr, bold2t1w_dof, omp_nthreads, name='bbreg_wf'):
     transforms = pe.Node(niu.Merge(2), run_without_submitting=True, name='transforms')
     reports = pe.Node(niu.Merge(2), run_without_submitting=True, name='reports')
 
-    lta_ras2ras = pe.MapNode(fs.utils.LTAConvert(out_lta=True), iterfield=['in_lta'],
+    lta_ras2ras = pe.MapNode(LTAConvert(out_lta=True), iterfield=['in_lta'],
                              name='lta_ras2ras', mem_gb=2)
     compare_transforms = pe.Node(niu.Function(function=compare_xforms), name='compare_transforms')
 
@@ -659,7 +663,7 @@ def init_fsl_bbr_wf(use_bbr, bold2t1w_dof, name='fsl_bbr_wf'):
     select_transform = pe.Node(niu.Select(), run_without_submitting=True, name='select_transform')
     select_report = pe.Node(niu.Select(), run_without_submitting=True, name='select_report')
 
-    fsl_to_lta = pe.MapNode(fs.utils.LTAConvert(out_lta=True), iterfield=['in_fsl'],
+    fsl_to_lta = pe.MapNode(LTAConvert(out_lta=True), iterfield=['in_fsl'],
                             name='fsl_to_lta')
 
     workflow.connect([

fmriprep/workflows/bold/resampling.py

@@ -27,7 +27,8 @@
 from ...interfaces.freesurfer import (
     MedialNaNs,
     # See https://github.com/poldracklab/fmriprep/issues/768
-    PatchedConcatenateLTA as ConcatenateLTA
+    PatchedConcatenateLTA as ConcatenateLTA,
+    PatchedLTAConvert as LTAConvert
 )
 from ..anatomical import TEMPLATE_MAP
 
@@ -116,7 +117,7 @@ def select_target(subject_id, space):
                             mem_gb=DEFAULT_MEMORY_MIN_GB)
     rename_src.inputs.subject = spaces
 
-    resampling_xfm = pe.Node(fs.utils.LTAConvert(in_lta='identity.nofile', out_lta=True),
+    resampling_xfm = pe.Node(LTAConvert(in_lta='identity.nofile', out_lta=True),
                              name='resampling_xfm')
     set_xfm_source = pe.Node(ConcatenateLTA(out_type='RAS2RAS'), name='set_xfm_source')