[WIP] Add state/mappers to new workflow syntax #2648
Conversation
…anging State.state_value so it returns values (and not arrays)
… SubmitterWorkflow class; simple test for NewNode.run using SubmitterNode
… had to create a new class and have now NewNodeBase and NewNode that is a wrapper with the run method
…be keep the order)
… to support it anyway)
…can use it for NewWorkflow; creating prepare_state_input in State from the __init__, so I can initialize State before having input
…dd_nodes, that takes nodes with mapper, tested; didnt work on map method; resume using state_inputs (since this is the inputs that I have before a workflow run and I can specify state_values based on it, etc.)
djarecka
changed the title
|
This is no longer compatible with mine and it's hard to see the changes. I'm going to push to upstream so that you can do a PR against mine. |
…gies-enh/workflow_syntax
Codecov Report
@@ Coverage Diff @@
## enh/workflow_syntax #2648 +/- ##
======================================================
Coverage ? 64.48%
======================================================
Files ? 343
Lines ? 43720
Branches ? 5513
======================================================
Hits ? 28192
Misses ? 14430
Partials ? 1098
Continue to review full report at Codecov.
|
effigies
changed the title
|
@effigies - not completely sure what do you want me to do. I can see that you actually committed to this PR, so do you still want me to create PR to your fork? |
|
No, I updated this. You should be able to |
|
@effigies - got it, pulled all changes |
|
@mgxd - do you know where I can find an example that used FreeSurfer? I already have access to my account on openmind, so you can just provide the path. |
|
I'm finally getting back to this. I'm sorry to keep being confused by simple things, but I can't really tell what the open problems are at this point. What kind of input are you hoping for? |
|
@effigies - right now I'm changing a few important things in Workflow, since I already started it might be better for now to wait till I finish and push changes. I'm also trying to produce input for the Satra example. |
|
@effigies - I run |
|
I've realized that I'm not sure if I chose the right |
|
Sorry, it should have been You can use whatever output spaces you want, as long as they exist in your FreeSurfer directory. |
… changing output/result in Node and Workflow
…checking inputs/outputs, changing submitter; finishing mapper for workflow: adding inner nodes, parent workflow is used to collect results, changing submitter; reorganization of SubmitterWorkflow (still have both: SubmitterNode and SubmitterWorkflow)
…east temporarily)
|
|
||
| wf.add(runnable=select_target, name="targets", subject_id="subject_id")\ | ||
| .map(mapper="space", inputs={"space": [space for space in Inputs["output_spaces"] | ||
| if space.startswith("fs")]}) |
There was a problem hiding this comment.
I think if we switch to a generator, we could do:
.map("space", space=(space for space in wf.inputs.output_spaces
if space.startswith('fs')))That would allow us to lazily evaluate wf.inputs.output_spaces at expansion time.
|
|
||
| #dj: don't have option in map to connect with wf input | ||
|
|
||
| wf.add(runnable=select_target, name="targets", subject_id="subject_id")\ |
There was a problem hiding this comment.
Is it possible to go back to @satra's syntax from #2539 (comment):
wf.add('targets', select_target, subject_id=wf.inputs.subject_id)\I know the last will require that wf.inputs attributes be futures, so leaving that as a string for now is fine. But it would make the protocol:
class Workflow:
...
def add(self, name, runnable, **input_map):
...By the way, I think it might be more consistent to use 'inputs.subject_id', treating wf.inputs as a node. Since we're using wf.add, the method should know the namespace we're working in.
| wf.add(name='rename_src', | ||
| runnable=Rename(format_string='%(subject)s', keep_ext=True), | ||
| in_file="source_file", subject="subject_id")\ | ||
| .map('subject') |
There was a problem hiding this comment.
I think here what we actually want is:
wf.add('rename_src', Rename(format_string='%(subject)s', keep_ext=True),
in_file="inputs.source_file", subject="select_target.out")The mapping is already being done at select_target. Since these are no longer MapNodes, we don't need to re-map unless we join first.
|
@effigies - recently I've mostly working on the engine itself, but today I'll also try to work on this example. Have to merge current interfaces/nodes with my new workflow. |
|
Cool. If there are engine-related things you'd like input on, could you comment on the relevant tests? |
… wf nodes; changing some names
…workingdir for Node and simplify the code
… adding flag that says if values or indecies are used in directories names and output
…_ci_output that reads output path from results*.pklz; adding print_val flag to Node/Workflow that says if an input value (or index) should be used in directories name; adding out_read flag to FunctionInterface that says if the value from the file should be saved in the output (instead of path)
|
This is all moved to pydra. |
adding my mapping and submitter to #2629. Had problem with merging NewNode with my submitter without creating an extra class NewNodeBase and treating NewNode as a wrapper that runs nodes's interface.
NeeWorkflows inherits from NewNode.
For now, I only added things related mapper, ignored join and joindByKey.