From d088cc232ca758554614295ed866eefe004dde80 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Wed, 8 Mar 2017 21:48:55 -0500 Subject: [PATCH 1/7] FIX: Make Eddy._use_cuda update instance variable Follow-up to #1711 --- nipype/interfaces/fsl/epi.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index 2a54ec14b4..60389419ad 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -525,9 +525,9 @@ def _num_threads_update(self): def _use_cuda(self): if self.inputs.use_cuda: - _cmd = 'eddy_cuda' + self._cmd = 'eddy_cuda' else: - _cmd = 'eddy_openmp' + self._cmd = 'eddy_openmp' def _format_arg(self, name, spec, value): if name == 'in_topup_fieldcoef': From c9a3b5eb51e07d5b915c9b0e497a36b1d46bca86 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Thu, 9 Mar 2017 10:08:40 -0500 Subject: [PATCH 2/7] Dynamically update _cmd property --- nipype/interfaces/fsl/epi.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index 60389419ad..bd6ae71795 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -498,7 +498,6 @@ class Eddy(FSLCommand): >>> res = eddy.run() # doctest: +SKIP """ - _cmd = 'eddy_openmp' input_spec = EddyInputSpec output_spec = EddyOutputSpec @@ -507,8 +506,6 @@ class Eddy(FSLCommand): def __init__(self, **inputs): super(Eddy, self).__init__(**inputs) self.inputs.on_trait_change(self._num_threads_update, 'num_threads') - if isdefined(self.inputs.use_cuda): - self._use_cuda() if not isdefined(self.inputs.num_threads): self.inputs.num_threads = self._num_threads else: @@ -523,11 +520,10 @@ def _num_threads_update(self): self.inputs.environ['OMP_NUM_THREADS'] = str( self.inputs.num_threads) - def _use_cuda(self): - if self.inputs.use_cuda: - self._cmd = 'eddy_cuda' - else: - self._cmd = 'eddy_openmp' + @property + def _cmd(self): + cuda = self.inputs.use_cuda + return 'eddy_cuda' if isdefined(cuda) and cuda else 'eddy_openmp' def _format_arg(self, name, spec, value): if name == 'in_topup_fieldcoef': From 772f540b3edc935f6f88747e9c15f3fb87ba0c03 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 20 Mar 2017 09:10:37 -0400 Subject: [PATCH 3/7] Find eddy command in FSL_BIN --- nipype/interfaces/fsl/epi.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index bd6ae71795..fe58582b36 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -520,10 +520,25 @@ def _num_threads_update(self): self.inputs.environ['OMP_NUM_THREADS'] = str( self.inputs.num_threads) + @staticmethod + def _eddycmd(use_cuda): + if 'FSL_BIN' not in os.environ: + raise RuntimeError("eddy command requires environment variable " + "FSL_BIN to be set") + FSL_BIN = os.environ['FSL_BIN'] + if use_cuda and os.path.exists(os.path.join(FSL_BIN, 'eddy_cuda')): + return 'eddy_cuda' + elif os.path.exists(os.path.join(FSL_BIN, 'eddy_openmp')): + return 'eddy_openmp' + elif os.path.exists(os.path.join(FSL_BIN, 'eddy')): + return 'eddy' + raise RuntimeError("eddy command not found in FSL_BIN: " + "'{}'".format(FSL_BIN)) + @property def _cmd(self): - cuda = self.inputs.use_cuda - return 'eddy_cuda' if isdefined(cuda) and cuda else 'eddy_openmp' + return self._eddycmd(isdefined(self.inputs.use_cuda) and + self.inputs.use_cuda) def _format_arg(self, name, spec, value): if name == 'in_topup_fieldcoef': From 5ab828d3c0e9d42af9dd2dcd3b93c13d6489494f Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 20 Mar 2017 09:11:04 -0400 Subject: [PATCH 4/7] PEP8 cleanup --- nipype/interfaces/fsl/epi.py | 59 +++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 28 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index fe58582b36..e6cbc6792a 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -12,7 +12,8 @@ ... '../../testing/data')) >>> os.chdir(datadir) """ -from __future__ import print_function, division, unicode_literals, absolute_import +from __future__ import print_function, division, unicode_literals, \ + absolute_import from builtins import str import os @@ -106,7 +107,7 @@ def _run_interface(self, runtime): out_file = self.inputs.out_fieldmap im = nb.load(out_file, mmap=NUMPY_MMAP) dumb_img = nb.Nifti1Image(np.zeros(im.shape), im.affine, - im.header) + im.header) out_nii = nb.funcs.concat_images((im, dumb_img)) nb.save(out_nii, out_file) @@ -450,26 +451,25 @@ class EddyInputSpec(FSLCommandInputSpec): class EddyOutputSpec(TraitedSpec): - out_corrected = File(exists=True, - desc=('4D image file containing all the corrected ' - 'volumes')) - out_parameter = File(exists=True, - desc=('text file with parameters definining the ' - 'field and movement for each scan')) - out_rotated_bvecs = File(exists=True, - desc=('File containing rotated b-values for all volumes')) - out_movement_rms = File(exists=True, - desc=('Summary of the "total movement" in each volume')) - out_restricted_movement_rms = File(exists=True, - desc=('Summary of the "total movement" in each volume ' - 'disregarding translation in the PE direction')) - out_shell_alignment_parameters = File(exists=True, - desc=('File containing rigid body movement parameters ' - 'between the different shells as estimated by a ' - 'post-hoc mutual information based registration')) - out_outlier_report = File(exists=True, - desc=('Text-file with a plain language report ' - 'on what outlier slices eddy has found')) + out_corrected = File( + exists=True, desc='4D image file containing all the corrected volumes') + out_parameter = File( + exists=True, desc=('text file with parameters definining the field and' + 'movement for each scan')) + out_rotated_bvecs = File( + exists=True, desc='File containing rotated b-values for all volumes') + out_movement_rms = File( + exists=True, desc='Summary of the "total movement" in each volume') + out_restricted_movement_rms = File( + exists=True, desc=('Summary of the "total movement" in each volume ' + 'disregarding translation in the PE direction')) + out_shell_alignment_parameters = File( + exists=True, desc=('File containing rigid body movement parameters ' + 'between the different shells as estimated by a ' + 'post-hoc mutual information based registration')) + out_outlier_report = File( + exists=True, desc=('Text-file with a plain language report on what ' + 'outlier slices eddy has found')) class Eddy(FSLCommand): @@ -492,9 +492,9 @@ class Eddy(FSLCommand): >>> eddy.inputs.in_bvec = 'bvecs.scheme' >>> eddy.inputs.in_bval = 'bvals.scheme' >>> eddy.cmdline # doctest: +ELLIPSIS +ALLOW_UNICODE - 'eddy_openmp --acqp=epi_acqp.txt --bvals=bvals.scheme --bvecs=bvecs.scheme \ ---imain=epi.nii --index=epi_index.txt --mask=epi_mask.nii \ ---out=.../eddy_corrected' + 'eddy_openmp --acqp=epi_acqp.txt --bvals=bvals.scheme \ +--bvecs=bvecs.scheme --imain=epi.nii --index=epi_index.txt \ +--mask=epi_mask.nii --out=.../eddy_corrected' >>> res = eddy.run() # doctest: +SKIP """ @@ -562,7 +562,8 @@ def _list_outputs(self): out_restricted_movement_rms = os.path.abspath( '%s.eddy_restricted_movement_rms' % self.inputs.out_base) out_shell_alignment_parameters = os.path.abspath( - '%s.eddy_post_eddy_shell_alignment_parameters' % self.inputs.out_base) + '%s.eddy_post_eddy_shell_alignment_parameters' + % self.inputs.out_base) out_outlier_report = os.path.abspath( '%s.eddy_outlier_report' % self.inputs.out_base) @@ -571,9 +572,11 @@ def _list_outputs(self): if os.path.exists(out_movement_rms): outputs['out_movement_rms'] = out_movement_rms if os.path.exists(out_restricted_movement_rms): - outputs['out_restricted_movement_rms'] = out_restricted_movement_rms + outputs['out_restricted_movement_rms'] = \ + out_restricted_movement_rms if os.path.exists(out_shell_alignment_parameters): - outputs['out_shell_alignment_parameters'] = out_shell_alignment_parameters + outputs['out_shell_alignment_parameters'] = \ + out_shell_alignment_parameters if os.path.exists(out_outlier_report): outputs['out_outlier_report'] = out_outlier_report From e42d90601ed2aa865e6be296f4d07b4ba7e9da99 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 20 Mar 2017 10:19:11 -0400 Subject: [PATCH 5/7] Use FSLDIR, warn instead of throwing exception --- nipype/interfaces/fsl/epi.py | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index e6cbc6792a..c9db9d03e2 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -21,6 +21,7 @@ import nibabel as nb import warnings +from ... import logging from ...utils.filemanip import split_filename from ...utils import NUMPY_MMAP @@ -522,18 +523,20 @@ def _num_threads_update(self): @staticmethod def _eddycmd(use_cuda): - if 'FSL_BIN' not in os.environ: - raise RuntimeError("eddy command requires environment variable " - "FSL_BIN to be set") - FSL_BIN = os.environ['FSL_BIN'] - if use_cuda and os.path.exists(os.path.join(FSL_BIN, 'eddy_cuda')): + logger = logging.getLogger('interface') + if 'FSLDIR' not in os.environ: + logger.warn("FSLDIR not set: assuming command 'eddy'") + return 'eddy' + + FSLDIR = os.environ['FSLDIR'] + if use_cuda and os.path.exists(os.path.join(FSLDIR, 'eddy_cuda')): return 'eddy_cuda' - elif os.path.exists(os.path.join(FSL_BIN, 'eddy_openmp')): + elif os.path.exists(os.path.join(FSLDIR, 'eddy_openmp')): return 'eddy_openmp' - elif os.path.exists(os.path.join(FSL_BIN, 'eddy')): - return 'eddy' - raise RuntimeError("eddy command not found in FSL_BIN: " - "'{}'".format(FSL_BIN)) + elif not os.path.exists(os.path.join(FSLDIR, 'eddy')): + logger.warn("No eddy binary found in FSLDIR; assuming command " + "'eddy'\nFSLDIR: '{}'".format(FSLDIR)) + return 'eddy' @property def _cmd(self): From 84bce1428c35ffb5a10a732fca2fcc2aee9157c4 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 20 Mar 2017 10:48:52 -0400 Subject: [PATCH 6/7] Revert Eddy cmdline in doctest --- nipype/interfaces/fsl/epi.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index c9db9d03e2..53f95b6599 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -493,9 +493,9 @@ class Eddy(FSLCommand): >>> eddy.inputs.in_bvec = 'bvecs.scheme' >>> eddy.inputs.in_bval = 'bvals.scheme' >>> eddy.cmdline # doctest: +ELLIPSIS +ALLOW_UNICODE - 'eddy_openmp --acqp=epi_acqp.txt --bvals=bvals.scheme \ ---bvecs=bvecs.scheme --imain=epi.nii --index=epi_index.txt \ ---mask=epi_mask.nii --out=.../eddy_corrected' + 'eddy --acqp=epi_acqp.txt --bvals=bvals.scheme --bvecs=bvecs.scheme \ +--imain=epi.nii --index=epi_index.txt --mask=epi_mask.nii \ +--out=.../eddy_corrected' >>> res = eddy.run() # doctest: +SKIP """ From 33f5db1f2e8bd7d80b1c850a272012237bf33aae Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 22 Mar 2017 09:53:14 -0400 Subject: [PATCH 7/7] Replace eddy_openmp at runtime if not in FSLDIR Set _use_cuda to run on trait change Update doctest to show use_cuda behavior --- nipype/interfaces/fsl/epi.py | 50 +++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/nipype/interfaces/fsl/epi.py b/nipype/interfaces/fsl/epi.py index 53f95b6599..2b45d62c2d 100644 --- a/nipype/interfaces/fsl/epi.py +++ b/nipype/interfaces/fsl/epi.py @@ -21,7 +21,6 @@ import nibabel as nb import warnings -from ... import logging from ...utils.filemanip import split_filename from ...utils import NUMPY_MMAP @@ -492,13 +491,20 @@ class Eddy(FSLCommand): >>> eddy.inputs.in_acqp = 'epi_acqp.txt' >>> eddy.inputs.in_bvec = 'bvecs.scheme' >>> eddy.inputs.in_bval = 'bvals.scheme' + >>> eddy.inputs.use_cuda = True >>> eddy.cmdline # doctest: +ELLIPSIS +ALLOW_UNICODE - 'eddy --acqp=epi_acqp.txt --bvals=bvals.scheme --bvecs=bvecs.scheme \ + 'eddy_cuda --acqp=epi_acqp.txt --bvals=bvals.scheme --bvecs=bvecs.scheme \ --imain=epi.nii --index=epi_index.txt --mask=epi_mask.nii \ --out=.../eddy_corrected' + >>> eddy.inputs.use_cuda = False + >>> eddy.cmdline # doctest: +ELLIPSIS +ALLOW_UNICODE + 'eddy_openmp --acqp=epi_acqp.txt --bvals=bvals.scheme \ +--bvecs=bvecs.scheme --imain=epi.nii --index=epi_index.txt \ +--mask=epi_mask.nii --out=.../eddy_corrected' >>> res = eddy.run() # doctest: +SKIP """ + _cmd = 'eddy_openmp' input_spec = EddyInputSpec output_spec = EddyOutputSpec @@ -511,6 +517,9 @@ def __init__(self, **inputs): self.inputs.num_threads = self._num_threads else: self._num_threads_update() + self.inputs.on_trait_change(self._use_cuda, 'use_cuda') + if isdefined(self.inputs.use_cuda): + self._use_cuda() def _num_threads_update(self): self._num_threads = self.inputs.num_threads @@ -521,27 +530,22 @@ def _num_threads_update(self): self.inputs.environ['OMP_NUM_THREADS'] = str( self.inputs.num_threads) - @staticmethod - def _eddycmd(use_cuda): - logger = logging.getLogger('interface') - if 'FSLDIR' not in os.environ: - logger.warn("FSLDIR not set: assuming command 'eddy'") - return 'eddy' - - FSLDIR = os.environ['FSLDIR'] - if use_cuda and os.path.exists(os.path.join(FSLDIR, 'eddy_cuda')): - return 'eddy_cuda' - elif os.path.exists(os.path.join(FSLDIR, 'eddy_openmp')): - return 'eddy_openmp' - elif not os.path.exists(os.path.join(FSLDIR, 'eddy')): - logger.warn("No eddy binary found in FSLDIR; assuming command " - "'eddy'\nFSLDIR: '{}'".format(FSLDIR)) - return 'eddy' - - @property - def _cmd(self): - return self._eddycmd(isdefined(self.inputs.use_cuda) and - self.inputs.use_cuda) + def _use_cuda(self): + self._cmd = 'eddy_cuda' if self.inputs.use_cuda else 'eddy_openmp' + + def _run_interface(self, runtime): + # If 'eddy_openmp' is missing, use 'eddy' + FSLDIR = os.getenv('FSLDIR', '') + cmd = self._cmd + if all((FSLDIR != '', + cmd == 'eddy_openmp', + not os.path.exists(os.path.join(FSLDIR, cmd)))): + self._cmd = 'eddy' + runtime = super(Eddy, self)._run_interface(runtime) + + # Restore command to avoid side-effects + self._cmd = cmd + return runtime def _format_arg(self, name, spec, value): if name == 'in_topup_fieldcoef':