Skip to content

FIX: Correctly populate output spec for ICA_AROMA #2027

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

Merged
merged 1 commit into from
May 18, 2017
Merged

FIX: Correctly populate output spec for ICA_AROMA #2027

merged 1 commit into from
May 18, 2017

Conversation

jdkent
Copy link
Contributor

@jdkent jdkent commented May 17, 2017

Fixes #2026

Changes proposed in this pull request
added outputs = self.output_spec().get to _list_outputs
May or may not fix the other error message.

@effigies effigies changed the title Closes #2026 FIX: Correctly populate output spec for ICA_AROMA May 17, 2017
@effigies
Copy link
Member

@jdkent Can you verify that you are able to run ICA_AROMA with this interface?

@jdkent
Copy link
Contributor Author

jdkent commented May 17, 2017

@effigies Sure, what is the best way to do this? copy terminal output?

@effigies
Copy link
Member

If you like. Just for you to verify that you can run it without errors is the main thing I'm looking for. So something like:

>>> aroma = ICA_AROMA(...)
>>> res = aroma.run()
>>> res.outputs
{'out_dir': <out_dir>, ...}

@satra
Copy link
Member

satra commented May 17, 2017

and this version for a workflow execution:

>>> from nipype import Node
>>> aroma = Node(ICA_AROMA(...), name="icaaroma")
>>> res = aroma.run()
>>> res.result.outputs
{'out_dir': <out_dir>, ...}

@jdkent
Copy link
Contributor Author

jdkent commented May 17, 2017

Okay, this did not fix the problem:
from @effigies's command construction, the outputs are undefined and are not in a dictionary format. I presume that @satra's command will fail to give me outputs too.

`>>> res.outputs

aggr_denoised_file = <undefined>
nonaggr_denoised_file = <undefined>
out_dir = <undefined>`

@effigies
Copy link
Member

effigies commented May 17, 2017

Can you show the full command sequence you gave? I'm surprised by that result.

Edit: Oh, if you want a dictionary, I think you need res.outputs.get().

@jdkent
Copy link
Contributor Author

jdkent commented May 17, 2017

I'm looking at how the CommandLine interface is defined, but I can't seem to figure out what's going wrong.

Here's how I set up the command.

>>> import nipype.interfaces.fsl.ICA_AROMA
>>> icaaroma = nipype.interfaces.fsl.ICA_AROMA.ICA_AROMA()
>>> icaaroma.inputs.in_file='/home/james/RestingState_dev/FLANKER/inputs/func/pre_sub10_FLANKER_RPI.nii.gz'
>>> icaaroma.inputs.motion_parameters='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/mc/mcImg.par'
>>> icaaroma.inputs.mat_file='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/functoT1.mat'
>>> icaaroma.inputs.fnirt_warp_file='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/T1toMNI_warp.nii.gz'
>>> icaaroma.inputs.denoise_type='nonaggr'
>>> icaaroma.inputs.out_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout11/'
>>> icaaroma.inputs.melodic_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout/melodic.ica'
>>> res = icaaroma.run()
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:------------------------------- RUNNING ICA-AROMA ------------------------------- 
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:--------------- 'ICA-based Automatic Removal Of Motion Artifacts' --------------- 
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:Step 1) MELODIC
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:  - The existing/specified MELODIC directory will be used.
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:Step 2) Automatic classification of the components
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:  - registering the spatial maps to MNI
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:  - extracting the CSF & Edge fraction features
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:  - extracting the Maximum RP correlation feature
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:  - extracting the High-frequency content feature
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:  - classification
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:Step 3) Data denoising
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:----------------------------------- Finished -----------------------------------
170517-15:07:43,19 interface INFO:
	 stdout 2017-05-17T15:07:43.018913:
>>> res.outputs

aggr_denoised_file = <undefined>
nonaggr_denoised_file = <undefined>
out_dir = <undefined>

@codecov-io
Copy link

codecov-io commented May 17, 2017

Codecov Report

Merging #2027 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2027      +/-   ##
=========================================
- Coverage    72.2%   72.2%   -0.01%     
=========================================
  Files        1132    1132              
  Lines       57023   57024       +1     
  Branches     8165    8165              
=========================================
  Hits        41173   41173              
- Misses      14566   14567       +1     
  Partials     1284    1284
Flag Coverage Δ
#smoketests 72.2% <0%> (-0.01%) ⬇️
#unittests 69.79% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/fsl/ICA_AROMA.py 75% <0%> (-2.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f92a333...69491b6. Read the comment docs.

@codecov-io
Copy link

Codecov Report

Merging #2027 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2027      +/-   ##
=========================================
- Coverage    72.2%   72.2%   -0.01%     
=========================================
  Files        1132    1132              
  Lines       57023   57024       +1     
  Branches     8165    8165              
=========================================
  Hits        41173   41173              
- Misses      14566   14567       +1     
  Partials     1284    1284
Flag Coverage Δ
#smoketests 72.2% <0%> (-0.01%) ⬇️
#unittests 69.79% <0%> (-0.01%) ⬇️
Impacted Files Coverage Δ
nipype/interfaces/fsl/ICA_AROMA.py 75% <0%> (-2.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f92a333...69491b6. Read the comment docs.

@effigies
Copy link
Member

Nothing's jumping out at me as to why that's not working. I might need to look with fresh eyes tomorrow. If somebody else does see something, can you help @jdkent?

@jdkent
Copy link
Contributor Author

jdkent commented May 17, 2017

@effigies I will help. Nothing is jumping out at me either, but I'll keep looking.

@jdkent
Copy link
Contributor Author

jdkent commented May 17, 2017

Many apologies, the version of nipype I was using with my virtual environment wasn't the most up to date version. After squaring that away I got the correct output with @effigies code structure

>>> import nipype.interfaces.fsl.ICA_AROMA
>>> icaaroma = nipype.interfaces.fsl.ICA_AROMA.ICA_AROMA()
>>> icaaroma.inputs.in_file='/home/james/RestingState_dev/FLANKER/inputs/func/pre_sub10_FLANKER_RPI.nii.gz'
>>> icaaroma.inputs.motion_parameters='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/mc/mcImg.par'
>>> icaaroma.inputs.mat_file='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/functoT1.mat'
>>> icaaroma.inputs.fnirt_warp_file='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/T1toMNI_warp.nii.gz'
>>> icaaroma.inputs.denoise_type='nonaggr'
>>> icaaroma.inputs.out_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout12/'
>>> icaaroma.inputs.melodic_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout/melodic.ica'
>>> icaaroma.inputs.out_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout18/'
>>> res = icaaroma.run()
170517-17:47:22,960 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:------------------------------- RUNNING ICA-AROMA ------------------------------- 
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:--------------- 'ICA-based Automatic Removal Of Motion Artifacts' --------------- 
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:Step 1) MELODIC
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:  - The existing/specified MELODIC directory will be used.
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:Step 2) Automatic classification of the components
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:  - registering the spatial maps to MNI
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:  - extracting the CSF & Edge fraction features
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:  - extracting the Maximum RP correlation feature
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:  - extracting the High-frequency content feature
170517-17:47:22,977 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:  - classification
170517-17:47:22,978 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:Step 3) Data denoising
170517-17:47:22,978 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:
170517-17:47:22,978 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:----------------------------------- Finished -----------------------------------
170517-17:47:22,978 interface INFO:
	 stdout 2017-05-17T17:47:22.960017:
>>> res.outputs

aggr_denoised_file = <undefined>
nonaggr_denoised_file = /home/james/RestingState_dev/FLANKER/ICA_AROMA_testout18/denoised_func_data_nonaggr.nii.gz
out_dir = /home/james/RestingState_dev/FLANKER/ICA_AROMA_testout18

>>> res.outputs.get(
... )
{'aggr_denoised_file': <undefined>, 'nonaggr_denoised_file': u'/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout18/denoised_func_data_nonaggr.nii.gz', 'out_dir': '/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout18'}
>>> 

However, I did not get the behavior expected by @satra when constructing a node. There was no result attribute, but there still was the outputs attribute. Is it because of how I imported Node?

>>> import nipype.interfaces.fsl.ICA_AROMA
>>> import nipype.pipeline.engine as pe
>>> icaaroma = pe.Node(nipype.interfaces.fsl.ICA_AROMA.ICA_AROMA(), name='icaaromanode')
>>> icaaroma.inputs.in_file='/home/james/RestingState_dev/FLANKER/inputs/func/pre_sub10_FLANKER_RPI.nii.gz'
>>> icaaroma.inputs.motion_parameters='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/mc/mcImg.par'
>>> icaaroma.inputs.mat_file='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/functoT1.mat'
>>> icaaroma.inputs.fnirt_warp_file='/home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/T1toMNI_warp.nii.gz'
>>> icaaroma.inputs.denoise_type='nonaggr'
>>> icaaroma.inputs.out_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout5/'
>>> icaaroma.inputs.melodic_dir='/home/james/RestingState_dev/FLANKER/ICA_AROMA_testout/melodic.ica'
>>> res = icaaroma.run()
170517-17:50:40,553 workflow INFO:
	 Executing node icaaromanode in dir: /tmp/tmp5fE7pL/icaaromanode
170517-17:50:40,565 workflow INFO:
	 Running: ICA_AROMA.py -den nonaggr -warp /home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/T1toMNI_warp.nii.gz -i /home/james/RestingState_dev/FLANKER/inputs/func/pre_sub10_FLANKER_RPI.nii.gz -affmat /home/james/RestingState_dev/FLANKER/BetaSeries_testout/reg/functoT1.mat -meldir /home/james/RestingState_dev/FLANKER/ICA_AROMA_testout/melodic.ica -mc /home/james/RestingState_dev/FLANKER/BetaSeries_testout/mc/mcImg.par -o /home/james/RestingState_dev/FLANKER/ICA_AROMA_testout5/
170517-18:02:31,726 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:
170517-18:02:31,726 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:------------------------------- RUNNING ICA-AROMA ------------------------------- 
170517-18:02:31,726 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:--------------- 'ICA-based Automatic Removal Of Motion Artifacts' --------------- 
170517-18:02:31,726 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:
170517-18:02:31,726 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:Step 1) MELODIC
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:  - The existing/specified MELODIC directory will be used.
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:Step 2) Automatic classification of the components
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:  - registering the spatial maps to MNI
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:  - extracting the CSF & Edge fraction features
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:  - extracting the Maximum RP correlation feature
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:  - extracting the High-frequency content feature
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:  - classification
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:Step 3) Data denoising
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:----------------------------------- Finished -----------------------------------
170517-18:02:31,727 interface INFO:
	 stdout 2017-05-17T18:02:31.726480:
>>> res.outputs

aggr_denoised_file = <undefined>
nonaggr_denoised_file = /home/james/RestingState_dev/FLANKER/ICA_AROMA_testout5/denoised_func_data_nonaggr.nii.gz
out_dir = /home/james/RestingState_dev/FLANKER/ICA_AROMA_testout5

>>> res.result.outputs
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'InterfaceResult' object has no attribute 'result'

@satra
Copy link
Member

satra commented May 18, 2017

@jdkent - my instructions were off. i meant to say node.result.outputs. but this output looks fine.

@satra satra merged commit 90cfdd8 into nipy:master May 18, 2017
@jdkent jdkent mentioned this pull request May 18, 2017
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 this pull request may close these issues.

ICA_AROMA() not working
4 participants