EPI-derived workflows and interfaces revisited #662

oesteban · 2013-09-18T05:14:01Z

I refactored the structure, added new interfaces ( PrepareFieldmap, TOPUP, ApplyTOPUP) and corresponding workflows ( fieldmap_correct and topup_correct )

A new preprocess file will contain all workflows regarding preprocessing of EPI images.

Since I'm to replace this workflow, I rename it with its ``classic'' name. The new pipeline will be based on the latest versions of ```fsl_prepare``` and ```epi_reg``` of FSL. Additionally, my intention is to provide with an interface for the new tool ```topup``` and a new workflow for T2-to-B0 registration-based correction.

Created a new file for adding EPI-related interfaces.

Added base classes for interfaces fsl_prepare_fieldmap, topup and applytopup

Start testing

Still needed to check outputs

Susceptibility correction with fieldmap

Additionally: now fsl.PrepareFieldmap gives a 4D output, as it is requested by fugue.

Also, part of the ApplyTOPUP has been refined

Before proceeding to submit the PR. Close #661

Conflicts: nipype/interfaces/fsl/utils.py nipype/workflows/dmri/fsl/dti.py

satra · 2013-09-20T14:52:33Z

nipype/interfaces/fsl/epi.py

+            return self._list_outputs()['out_file']
+        return None
+
+class EddyCorrectInputSpec(FSLCommandInputSpec):


if you have time, could you also add eddy?

You mean, the new tool, right?

yes .. eddy was introduced to deal with the human connectome project data

ok, it looks similar to applytopup interface, I think I can make it. At least a base version if I found something difficult to implement (e.i. there were 3 parameters in topup that are not interfaced because I don't know exactly how they work).

feel free to ask the FSL mailing list for clarification if their latest documentation here doesn't explain things.

http://fsl.fmrib.ox.ac.uk/fsl/fslwiki/TOPUP/TopupUsersGuide
http://fsl.fmrib.ox.ac.uk/fsl/fslwiki/Eddy/UsersGuide

In-place run of the standard tool. Ref nipy#662

Encoding direction and readout time were missing

satra · 2013-10-02T13:02:52Z

this is looking great - i'll try to run it on some data this week and will merge over the weekend. i'll send PR's to your branch before that.

beOn · 2013-10-02T13:28:33Z

Cool deal - let me know if it needs anything else!

On Oct 2, 2013, at 8:02 AM, Satrajit Ghosh notifications@github.com wrote:

this is looking great - i'll try to run it on some data this week and will merge over the weekend. i'll send PR's to your branch before that.

—
Reply to this email directly or view it on GitHub.

* New interface for eddy. NO TESTING HAS BEEN DONE, though * Added interface.cmdline on documentation comments

Added Eddy to __init__ Fixed minor typo in documentation comments.

oesteban · 2013-10-05T18:23:23Z

I think most of the requirements are met:

Added new FSL-eddy interface
Some fixes to TOPUP and associated workflow
Added interface.cmdline to documentation

Ready?

This reverts commit 5f5b978.

satra · 2013-10-08T04:03:00Z

@oesteban - there are several failing tests. please run: make test in the main directory.

oesteban · 2013-10-08T04:57:21Z

Ok, I'm currently running it. I'm sorry I did not know I should check for that.

This would fix one of the failed tests

oesteban · 2013-10-08T06:09:24Z

I started using Travis CI -> https://travis-ci.org/oesteban/nipype

I have a question: on those errors regarding interface.cmdline how should I fix it, keeping the cmdline as you requested? For the rest of them, I'll try to fix them ASAP. Below, an example of the cmdline errors:

FAIL: Doctest: nipype.interfaces.fsl.epi.ApplyTOPUP
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/doctest.py", line 2201, in runTest
    raise self.failureException(self.format_failure(new.getvalue()))
AssertionError: Failed doctest test for nipype.interfaces.fsl.epi.ApplyTOPUP
  File "/home/travis/build/oesteban/nipype/nipype/interfaces/fsl/epi.py", line 241, in ApplyTOPUP
----------------------------------------------------------------------
File "/home/travis/build/oesteban/nipype/nipype/interfaces/fsl/epi.py", line 256, in nipype.interfaces.fsl.epi.ApplyTOPUP
Failed example:
    applytopup.cmdline
Expected nothing
Got:
    'applytopup --datain=topup_encoding.txt --imain=epi.nii,epi_rev.nii --inindex=1,2 --topup=my_topup_results --out=/home/travis/build/oesteban/nipype/nipype/testing/data/nipypeatu'

Added skip command that I guess solve the problem

oesteban · 2013-10-08T06:17:39Z

Probably I had to add # doctest: +SKIP after each line in documentation that should not be executed, right?

create_dmri_preprocessing was still calling the epidewarp workflow with its former name

oesteban · 2013-10-08T06:41:06Z

A TODO task would be to update the workflow that uses fieldmap correction. It should be able to perform fieldmap with the new fsl_prepare interface and reverse encoding with TOPUP.

This reverts commit 8f08e1c. Conflicts: nipype/algorithms/misc.py This commit should not be here, sorry for that. If necessary, I will run autopep again on misc.py

satra · 2013-10-08T12:03:28Z

@oesteban : here is how to do the command line fixes -

https://github.com/nipy/nipype/blob/master/nipype/interfaces/ants/registration.py#L359

you paste the expected command line right after the interface.cmdline

thanks for switching to travis - it's a good way of running some parts of the tests - we will soon have a mechanism to test across versions and platforms.

Now, the tests are correctly implemented

oesteban · 2013-10-08T17:14:51Z

@satra , one last question: what happens with absolute paths (especially in outputs)... I implemented everything, but of course absolute paths don't match...

satra · 2013-10-08T17:19:37Z

interfaces typically generate things in either the current working directory (1) or in the directory of some input file (2). nodes will want to generate all output in their local directory. if (2) above, we add the following metadata to the inputspec for that input (copyfile=True/False - True copies, False symlinks).

then in gen_filename we will typically generate a name without the absolute path
and in list_outputs we would do os.path.abspath(generated_name)

oesteban · 2013-10-08T17:32:03Z

I never managed to use the gen_filename facilities... so I usually hard-code them (which I'm aware is not very nice). But still we are in the same problem: if I let the example automatically generate output names, then the test doesn't work... right?. The os.path.abspath(generated_name) in list_outputs breaks the test.

oesteban · 2013-10-11T16:45:14Z

Thank you all, I saw that I can use three dots (...) to get the abspath computed from an interface :)

satra · 2013-10-11T20:31:54Z

actually we really need to move towards more consistent file generation as available in PR #681

oesteban added 18 commits September 7, 2013 17:01

Base commit splitting the dti utilities

5ffd98c

A new preprocess file will contain all workflows regarding preprocessing of EPI images.

Extracted EPI-specific interfaces from utils & dti

b08c195

Created a new file for adding EPI-related interfaces.

Base files for new epi interfaces

75f919b

Added base classes for interfaces fsl_prepare_fieldmap, topup and applytopup

Prepare interface implemented

488e77a

Start testing

Finished fsl_prepare_fieldmap interface

08cae16

Added interface for topup

842cfb4

Fixed details in topup

73ebbd8

Still needed to check outputs

Added interface for applytopup

158051d

Fixed ApplyTOPUP and TOPUP

5bbfcf9

Added workflow using fsl 5.0 for correction

b25521a

Susceptibility correction with fieldmap

Finished and tested the new fieldmap_correction wf

d9bca64

Additionally: now fsl.PrepareFieldmap gives a 4D output, as it is requested by fugue.

Added support to set the encoding direction

5ffab9e

Added automatic generation of encfile in TOPUP

7327df0

Finished workflow for TOPUP

9e1d80d

Also, part of the ApplyTOPUP has been refined

Updated changelog

bb60a78

Before proceeding to submit the PR. Close #661

Merge branch 'master' into enh/NewPreprocessDMRIWorkflows

914a830

Conflicts: nipype/interfaces/fsl/utils.py nipype/workflows/dmri/fsl/dti.py

Merge branch 'bug/FixAntsInterface' into enh/NewPreprocessDMRIWorkflows

c1b2e94

satra reviewed Sep 20, 2013
View reviewed changes

beOn pushed a commit to beOn/nipype that referenced this pull request Sep 20, 2013

Run autopep8 on misc.py

8f08e1c

In-place run of the standard tool. Ref nipy#662

oesteban added 3 commits September 25, 2013 18:15

Added some useful outputs

a66d82a

Fixed name

bc3844a

Added missing connections to topup workflow

c0986c6

Encoding direction and readout time were missing

oesteban added 4 commits October 3, 2013 00:40

Improvements on TOPUP and related workflow

cbe7b72

New eddy interface (#662)

61bc69c

* New interface for eddy. NO TESTING HAS BEEN DONE, though * Added interface.cmdline on documentation comments

Added dumb files for interfaces

78fd9f6

Checked that eddy.cmdline works

2d24750

Added Eddy to __init__ Fixed minor typo in documentation comments.

Added Eddy to changelog

6f3e7c2

Revert "Changed n. bins"

984456a

This reverts commit 5f5b978.

Split fsl dti and epi tests

4e26bef

This would fix one of the failed tests

Fixed errors in Doctest (?)

4048dca

Added skip command that I guess solve the problem

Fixed failed test

0e2b7a3

create_dmri_preprocessing was still calling the epidewarp workflow with its former name

Revert "Run autopep8 on misc.py"

5d0837e

This reverts commit 8f08e1c. Conflicts: nipype/algorithms/misc.py This commit should not be here, sorry for that. If necessary, I will run autopep again on misc.py

Fixed cmdline commands in docstrings

bcd8a3f

Now, the tests are correctly implemented

Added nocheck option in fielmap_correction

6908420

chrisgorgo merged commit 6908420 into nipy:master Oct 11, 2013

oesteban deleted the enh/NewPreprocessDMRIWorkflows branch October 11, 2013 16:45

EPI-derived workflows and interfaces revisited #662

EPI-derived workflows and interfaces revisited #662

Uh oh!

Conversation

oesteban commented Sep 18, 2013

Uh oh!

satra Sep 20, 2013

Choose a reason for hiding this comment

Uh oh!

oesteban Sep 20, 2013

Choose a reason for hiding this comment

Uh oh!

satra Sep 20, 2013

Choose a reason for hiding this comment

Uh oh!

oesteban Sep 20, 2013

Choose a reason for hiding this comment

Uh oh!

satra Sep 20, 2013

Choose a reason for hiding this comment

Uh oh!

satra commented Oct 2, 2013

Uh oh!

beOn commented Oct 2, 2013

Uh oh!

oesteban commented Oct 5, 2013

Uh oh!

satra commented Oct 8, 2013

Uh oh!

oesteban commented Oct 8, 2013

Uh oh!

oesteban commented Oct 8, 2013

Uh oh!

oesteban commented Oct 8, 2013

Uh oh!

oesteban commented Oct 8, 2013

Uh oh!

satra commented Oct 8, 2013

Uh oh!

oesteban commented Oct 8, 2013

Uh oh!

satra commented Oct 8, 2013

Uh oh!

oesteban commented Oct 8, 2013

Uh oh!

oesteban commented Oct 11, 2013

Uh oh!

satra commented Oct 11, 2013

Uh oh!

Uh oh!