Skip to content

Topup output not found #796

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
arnaudbore opened this issue Feb 10, 2014 · 16 comments
Closed

Topup output not found #796

arnaudbore opened this issue Feb 10, 2014 · 16 comments

Comments

@arnaudbore
Copy link

Dear nipype experts,

When I run topup it looks for a ".nii.gz" file as output 'out_fieldcoef'.
Because all my FSL settings are in ".nii" file it doesn't find anything and then crashes.

Thank you for your help.
Arnaud

@satra
Copy link
Member

satra commented Feb 10, 2014

this is being fixed here:

#794

@arnaudbore
Copy link
Author

Thank you Satra. Can you tell me when is going to be in the new version of nipype ? Maybe 0.9.3 ?

@satra
Copy link
Member

satra commented Feb 10, 2014

definitely in 0.9.3 - it should be in master soon. we'll see if we can release 0.9.3 by end of february. there are a few other things that need to happen as well.

@oesteban
Copy link
Contributor

The output 'out_fieldcoef' is assigned here: https://github.com/nipy/nipype/blob/master/nipype/interfaces/fsl/epi.py#L222 using the fsl.base method to generate filenames, as the argument change_ext is not overridden, it should observe the environment settings.

Is this happening to you with other interfaces of FSL?

@oesteban
Copy link
Contributor

oesteban commented Sep 1, 2014

@arnaudbore Did you find out the problem?

@satra
Copy link
Member

satra commented Sep 1, 2014

@oesteban - i think this was fixed in #794

@arnaudbore - did you check?

oesteban added a commit to oesteban/nipype that referenced this issue Sep 1, 2014
* TOPUP: regarding nipy#794, nipy#796 I've cleaned up the docstring and implemented
  the three arguments that were still missing. The problem is that TOPUP
  admits one or more values for them, so the implementation of all the
  traits should be updated.

* Eddy: the implementation of @JensNRAD seems to work out, I reviewed
  the docstring. Close nipy#769.

* Added deprecation notices and minor fixes.
@rfdougherty
Copy link

I don't think it's been fixed. I just pulled master and still get a FileNotFoundError because it's looking for the fieldcoef file in the current directory rather than the directory specified in out_base (which is where the fieldcoef file actually is).

@oesteban
Copy link
Contributor

That's a bright hypothesis, I did not think of out_base. I'm currently looking at this. Thanks @rfdougherty .

EDIT: ok, yes, a fast doctest writing out_base shows that the outputs are not updated.

@oesteban
Copy link
Contributor

@satra this is the diagnostic:

we have two input traits that should name_source from the same trait (out_base) that should be sourced itself from in_file. This dependency is broken by the sorted return of traits within _parse_inputs.

I've thought of two solutions:

  1. Review this sorted iteration of traits, so that dependencies are identified and the sources set in front of the list.

  2. Modify _filename_from_souce to be recursive. This is probably a faster fix, but also produces risky redundancies in parsing.

oesteban added a commit to oesteban/nipype that referenced this issue Sep 20, 2014
@oesteban
Copy link
Contributor

Submitted a fix, using option 2). If inputs are not changed during parsing, it should work out.

@satra
Copy link
Member

satra commented Sep 21, 2014

@rfdougherty and @oesteban - could you take a look at this notebook before we make any changes?

http://nbviewer.ipython.org/urls/dl.dropbox.com/s/l84fgl333fperqb/Eddy.ipynb?dl=0

this workflow ran fine for me. i'll run it again to make sure things haven't changed.

@rfdougherty
Copy link

Hi Satra,

Everything does work fine if cwd == out_base. Reading through your workflow, it seems that's how you run it. E.g., " merged_file = os . path . join ( os . getcwd (), 'merged.nii.gz' )". But it will fail if out_base != cwd.

cheers,
bob

Robert F. Dougherty, PhD
Research Director
Stanford Center for Cognitive and Neurobiological Imaging
70 Jordan Hall * Stanford CA 94305 * 650-725-0051
http://www.stanford.edu/~bobd

----- Original Message -----

From: "Satrajit Ghosh" [email protected]
To: "nipy/nipype" [email protected]
Cc: "Bob Dougherty" [email protected]
Sent: Sunday, September 21, 2014 6:20:24 AM
Subject: Re: [nipype] Topup output not found (#796)

@rfdougherty and @oesteban - could you take a look at this notebook before we make any changes?

http://nbviewer.ipython.org/urls/dl.dropbox.com/s/l84fgl333fperqb/Eddy.ipynb?dl=0

this workflow ran fine for me. i'll run it again to make sure things haven't changed.


Reply to this email directly or view it on GitHub .

@satra
Copy link
Member

satra commented Sep 22, 2014

thanks @rfdougherty:

narrowed it to this - the only thing that's different here is that out_base is specified as a location somewhere:

In [1]: from nipype.interfaces.fsl import TOPUP
In [2]: topup = TOPUP()
In [3]: topup.inputs.in_file = "b0_b0rev.nii"
In [4]: topup.inputs.encoding_file = "topup_encoding.txt"
In [5]: topup.cmdline
Out[5]: 'topup --config=b02b0.cnf --datain=topup_encoding.txt --imain=b0_b0rev.nii --out=b0_b0rev_base --iout=b0_b0rev_corrected.nii.gz --fout=b0_b0rev_field.nii.gz --logout=b0_b0rev_topup.log'

In [6]: topup.inputs.out_base = '/tmp/foo'
In [7]: topup.cmdline
Out[7]: 'topup --config=b02b0.cnf --datain=topup_encoding.txt --imain=b0_b0rev.nii --out=/tmp/foo --iout=b0_b0rev_corrected.nii.gz --fout=b0_b0rev_field.nii.gz --logout=b0_b0rev_topup.log'
In [8]: topup._list_outputs()
Out[8]: 
{'out_corrected': '/software/nipy-repo/nipype/nipype/testing/data/b0_b0rev_corrected.nii.gz',
 'out_enc_file': <undefined>,
 'out_field': '/software/nipy-repo/nipype/nipype/testing/data/b0_b0rev_field.nii.gz',
 'out_fieldcoef': '/software/nipy-repo/nipype/nipype/testing/data/foo_fieldcoef.nii.gz',
 'out_logfile': '/software/nipy-repo/nipype/nipype/testing/data/b0_b0rev_topup.log',
 'out_movpar': '/software/nipy-repo/nipype/nipype/testing/data/foo_movpar.txt'}

we need to do two things:

  1. check to ensure that fsl respects such a location specification. (i'll test this)
  2. depending on the answer modify topup. i don't think a base class modification is required here.

@satra
Copy link
Member

satra commented Sep 22, 2014

results from 1. topup doesn't appear to like it when out contains a location that doesn't exist. so we have to be careful how this interface gets redesigned. i'm now running it with the same command as below, but having created /tmp/foo

$ topup --config=b02b0.cnf --datain=/software/data/isn/eddy/preproc/b0_acq.txt --imain=/software/data/isn/eddy/preproc/b0_merged.nii.gz --numprec=float --out=/tmp/foo/b0_merged_base --iout=b0correct.nii.gz --fout=b0_merged_field.nii.gz --logout=b0_merged_topup.log
Error: failed to open file /tmp/foo/b0_merged_base_fieldcoef.nii.gz
Image Exception : #22 :: ERROR: Could not open image /tmp/foo/b0_merged_base_fieldcoef
libc++abi.dylib: terminating with uncaught exception of type RBD_COMMON::BaseException
Abort trap: 6

@satra
Copy link
Member

satra commented Sep 23, 2014

so this works:

$ topup --config=b02b0.cnf --datain=/software/data/isn/eddy/preproc/b0_acq.txt --imain=/software/data/isn/eddy/preproc/b0_merged.nii.gz --numprec=float --out=/tmp/foo/b0_merged_base --iout=b0correct.nii.gz --fout=b0_merged_field.nii.gz --logout=b0_merged_topup.log

but the root directory (/tmp/foo in this case) has to exist.

i'll move over to the PR for comments now, but i think this can be tackled with a simpler change.

@oesteban
Copy link
Contributor

oesteban commented Oct 9, 2014

I think this is fixed

@oesteban oesteban closed this as completed Oct 9, 2014
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

Successfully merging a pull request may close this issue.

4 participants