nipype icon indicating copy to clipboard operation
nipype copied to clipboard

[ENH/WIP] WorkflowInterface

Open oesteban opened this issue 8 years ago • 5 comments
trafficstars

This PR creates a new type of interface that wraps workflows into a nipype interface. A naive addendum has been done to the BaseWorkflow class, in order to post-process the executed graph to retrieve the outputs that were connected to an "outputnode". This addition only happens for synchronous runners. For now, only the Linear and MultiProc plugins are set as synchronous.

The inner workflow is then run using either Linear or Multiproc.

The doctest reads like:

    >>> from nipype.pipeline import engine as pe
    >>> from nipype.interfaces.utility.base import IdentityInterface
    >>> wf = pe.Workflow('test_workflow')
    >>> node1 = pe.Node(IdentityInterface(fields=['a', 'b']), name='inputnode')
    >>> func = 'def func(a, b): return a + b'
    >>> node2 = pe.Node(Function(input_names=['a', 'b'], output_names=['out'],
    ...                 function=func), name='strconcat')
    >>> node3 = pe.Node(IdentityInterface(fields=['out']), name='outputnode')
    >>> wf.connect([
    ...     (node1, node2, [('a', 'a'), ('b', 'b')]),
    ...     (node2, node3, [('out', 'out')])
    ... ])
    >>> wi = WorkflowInterface(workflow=wf)
    >>> wi.inputs.a = 'b input '
    >>> wi.inputs.b = 'was concatenated to a'
    >>> res = wi.run()
    >>> res.outputs.out
    'b input was concatenated to a'

Before this PR can be merged, these tasks should be ticked :

  • [x] Merge #1828 (this refactoring was a preliminary work to this PR)
  • [x] Write some additional documentation (besides the Function interface)
  • [ ] Pass a config object to the inner workflow (probably overwriting the stop_at_first_crash option to True, maybe @satra can think of other options that should be set and potential problems of this approach)
  • [x] Add more tests:
    • [x] workflow that fails for some traits setting errors
    • [x] workflow that fails for some runtime error (maybe a Function interface that just raises)
    • [x] embed the new interface in a mapnode
    • [x] run the interface with iterables

oesteban avatar Feb 22 '17 07:02 oesteban

@oesteban - the new api we are working on basically has the following subclassing structure Node -> Workflow, so by definition a Workflow can be a Node. we have a discussion with @effigies and @djarecka scheduled for tomorrow and hoping we have a draft to present after that.

we are considering input_map and output_map as parameters to Workflow that can basically route inputs and outputs, without creating explicit inputspec and outputspec identity nodes. by default input_map and output_map would be a flattened key_value object for all nodes of the workflow, but a user could constrain that space by explicitly specifying these mappings, or sub-selecting from them.

however, this would be a good interface to implement that functionality.

satra avatar Feb 22 '17 15:02 satra

Codecov Report

Merging #1835 into master will increase coverage by 0.81%. The diff coverage is 96.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1835      +/-   ##
==========================================
+ Coverage   72.31%   73.13%   +0.81%     
==========================================
  Files        1180     1064     -116     
  Lines       58885    53546    -5339     
  Branches     8474        0    -8474     
==========================================
- Hits        42585    39160    -3425     
+ Misses      14921    14386     -535     
+ Partials     1379        0    -1379
Flag Coverage Δ
#smoketests ?
#unittests 73.13% <96.84%> (+3.23%) :arrow_up:
Impacted Files Coverage Δ
nipype/pipeline/plugins/linear.py 92.1% <100%> (+3.53%) :arrow_up:
nipype/pipeline/plugins/base.py 65.76% <100%> (+7.24%) :arrow_up:
nipype/pipeline/plugins/multiproc.py 75.55% <100%> (-1.21%) :arrow_down:
nipype/interfaces/utility/__init__.py 100% <100%> (ø) :arrow_up:
nipype/pipeline/engine/workflows.py 79.47% <100%> (+0.68%) :arrow_up:
nipype/interfaces/utility/wrappers.py 91.83% <93.87%> (+6.65%) :arrow_up:
nipype/interfaces/utility/tests/test_wrappers.py 93.58% <97.36%> (+6.55%) :arrow_up:
nipype/interfaces/spm/model.py 41.76% <0%> (-29.35%) :arrow_down:
nipype/algorithms/rapidart.py 37.46% <0%> (-27.14%) :arrow_down:
nipype/interfaces/fsl/model.py 54.81% <0%> (-25.55%) :arrow_down:
... and 967 more

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 56b7c81...41388eb. Read the comment docs.

codecov-io avatar Feb 23 '17 03:02 codecov-io

@oesteban is this one orphaned?

effigies avatar Jan 21 '18 14:01 effigies

@effigies and @oesteban - i wouldn't mind seeing this in the 1.0 branch, but this would be explicitly supported in 2.0 engine.

satra avatar Jan 21 '18 17:01 satra

I can recheck in two weeks. If re-syncing is not too hard (better said: very easy), we can add it as a patch for 1.0. As Satra mentioned, this does not make sense in 2.0.

oesteban avatar Jan 23 '18 04:01 oesteban