Skip to content

issues with afni.allineate w/ solution (and possible issue with genfile parameter) #2499

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

Closed
vanandrew opened this issue Mar 19, 2018 · 5 comments
Milestone

Comments

@vanandrew
Copy link
Contributor

Summary

Hi all. Just started learning nipype, and it's been pretty great so far. I've only run into issues with the afni.allineate interface. I was able to fix it on my own, but I thought I should post an issue here for awareness (plus I'm not sure how to do a pull request...)

There are 2 problems I've encountered. The first is that afni.allineate consistently fails the hash check when placed in a workflow and always reruns. I think this is because the hash_files parameter needs to be set to False.

out_file = File(
    desc='output file from 3dAllineate',
    argstr='-prefix %s',
    genfile=True,
    hash_files=False, # <-- when this is added it starts working again
    xor=['allcostx']) 

Referencing the docs, I think this is an appropriate change:

hash_files
To be used with inputs that are defining output filenames. When this flag is set to false any Nipype will not try to hash any files described by this input. This is useful to avoid rerunning when the specified output file already exists and has changed.

The second issue is that the prefix (out_file) argument does not get generated when used in a workflow. This issue is also referenced here: #2216, but with a temporary workaround.

My understanding is that when genfile is set to True, the _gen_filename() method for the interface gets called when the user does not specify a value. Looking at the code, the _gen_filename() does seems to implemented, but not correctly:

def _gen_filename(self, name): # this gets called when `out_file` is not defined
    if name == 'out_file':
        return self._list_outputs()[name] # <-- we are returning the value of `out_file`, which is undefined...
    return None

So I changed it to this:

def _gen_filename(self, name): # this gets called when `out_file` is not defined
    if name == 'out_file':
        return self._gen_fname(self.inputs.in_file,op.dirname(self.inputs.in_file),suffix='_allineate')
    return None

Which should define the out_file to be in the same location and have the same name as the in_file, but with the '_allineate' suffix.

When I run this, the problem seems to be fixed, as the prefix is now defined:

[Node] Running "3dallineate_orig" ("nipype.interfaces.afni.preprocess.Allineate"), a CommandLine Interface with command:
3dAllineate -source /home/vana/Projects/p3/tmp/P3/_subject_id_sub-CTS200/3dallineate_orig/orig_out.nii.gz -prefix /home/vana/Projects/p3/tmp/P3/_subject_id_sub-CTS200/3dallineate_orig/orig_out_allineate.nii.gz -1Dmatrix_save FSorig.XFM.FS2MPR.aff12.1D -overwrite -base /home/vana/Projects/p3/dataset/sub-CTS200/anat/sub-CTS200_T1w.nii.gz

But downstream nodes don't seem to notice that the out_file is actually defined now... (Note: I have a node whose in_file is connected to the out_file of the allineate node)

 [Node] Error on "P3.3drefit1" (/home/vana/Projects/p3/tmp/P3/_subject_id_sub-CTS200/3drefit1)
Traceback (most recent call last):
  File "./preproc.py", line 293, in <module>
    wf.run()
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/workflows.py", line 602, in run
    runner.run(execgraph, updatehash=updatehash, config=self.config)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/plugins/linear.py", line 44, in run
    node.run(updatehash=updatehash)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 487, in run
    result = self._run_interface(execute=True)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 571, in _run_interface
    return self._run_command(execute)
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 638, in _run_command
    cmd = self._interface.cmdline
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 935, in cmdline
    self._check_mandatory_inputs()
  File "/home/vana/.local/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 389, in _check_mandatory_inputs
    raise ValueError(msg)
ValueError: Refit requires a value for input 'in_file'. For a list of required inputs, see Refit.help()

After looking at some of the fsl interfaces (and some trial-and-error), I figured out that the downstream nodes seem to be getting the information for their inputs from the _list_outputs method of the upstream node, and ignore _gen_filename. So when I do this:

def _gen_outfilename(self):
    out_file = self.inputs.out_file
    if not isdefined(out_file) and isdefined(self.inputs.in_file) and not isdefined(self.inputs.allcostx):
        out_file = op.abspath(self._gen_fname(self.inputs.in_file,op.dirname(self.inputs.in_file),suffix='_allineate'))
    return out_file

def _list_outputs(self):
    outputs = self.output_spec().get()

    outputs['out_file'] = self._gen_outfilename()

    # other stuff below this ...

def _gen_filename(self, name):
    if name == 'out_file':
        return self._gen_outfilename()
    return None

Everything works! But then the question becomes, what's the point of the genfile parameter if I have to define a method that checks if the input is defined anyway?

I'm not sure how to do a pull request to this repo, so I've attached my git patch below. (This is on nipype 1.0.1)
allineate.patch.tar.gz

@satra
Copy link
Member

satra commented Mar 19, 2018

@vanandrew - we are trying to slowly discourage the genfile option and instead use the name_source option with possibly overwriting extension with a function _overload_extension.

the afni interfaces (especially because of their complex extensions) have been slowly refined. so something like this should perhaps look like:

out_file = traits.Str(
    desc='output file from 3dAllineate',
    argstr='-prefix %s',
    name_source='in_file',
    name_template='%s_allineate',
    keep_extension=True,
    xor=['allcostx']) 

this should void the need (at least for out_file) any _gen_filename or _list_outputs function.

@satra
Copy link
Member

satra commented Mar 19, 2018

@vanandrew - regarding a PR, if you push a branch to your own repo, github will put a prompt to initiate a pull-request against the master branch or you can initiate it with the pull-request tab (on your fork of nipype, in the appropriate branch). the contributing document has recently been updated to help working through the steps.

@vanandrew
Copy link
Contributor Author

@satra - Ah, I see. Thanks for the info! I'll put my changes properly on a branch.

@vanandrew vanandrew reopened this Mar 20, 2018
@vanandrew
Copy link
Contributor Author

@satra - Using name_source/name_template seems to break one of the tests, as -prefix (out_file) is now always defined even with no user-specified value. The -allcostx parameter is in xor with the -prefix parameter (which I assume would remove it, but I guess it doesn't work that way...)

===================================================== FAILURES ======================================================
_______________________________ [doctest] nipype.interfaces.afni.preprocess.Allineate _______________________________
466     >>> allineate.cmdline
467     '3dAllineate -source functional.nii -prefix functional_allineate.nii -1Dmatrix_apply cmatrix.mat'
468     >>> res = allineate.run()  # doctest: +SKIP
469 
470     >>> from nipype.interfaces import afni
471     >>> allineate = afni.Allineate()
472     >>> allineate.inputs.in_file = 'functional.nii'
473     >>> allineate.inputs.reference = 'structural.nii'
474     >>> allineate.inputs.allcostx = 'out.allcostX.txt'
475     >>> allineate.cmdline
Expected:
    '3dAllineate -source functional.nii -base structural.nii -allcostx |& tee out.allcostX.txt'
Got:
    '3dAllineate -source functional.nii -prefix functional_allineate -base structural.nii -allcostx |& tee out.allcostX.txt'

I'll just stick with my original solution for now.

@salma1601
Copy link
Contributor

@vanandrew @satra
I had a similar issue and caching never worked with allineate, so my solution was to do the following changes
to the Allineate interface

    out_file = File(
        name_template='%s_allineated',
        desc='output file from 3dAllineate',
        argstr='-prefix %s',
        name_source='in_file',
        xor=['allcostx'])

    out_matrix = File(
        argstr='-1Dmatrix_save %s',
        name_template='%s_allineated.aff12.1D',
        name_source='in_file',
        keep_extension=False,
        desc='Save the transformation matrix for each volume.',
        xor=['in_matrix','allcostx'])

may be this can help fixing this issue ?

@effigies effigies added this to the 1.0.3 milestone Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants