Emit signals when running a child directly

[liamhuber]

@Tara-Lakshmipathy pointed out that running a particular node doesn't actually push downstream when those nodes are inside the scope of a parent. Indeed, he's right and there was no simple flag or whatever to enable this!

So here Node.run is modified to force signals to emit from nodes who don't have a parent (the existing behaviour) OR who do but whose parent is not itself currently running (and thus managing execution flow) (the new behaviour).

[github-actions]

👈 Launch a binder notebook on branch pyiron/pyiron_workflow/push

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ +0.01% (target: -1.00%)	✅ 100.00%

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (9075529)	3441	3147	91.46%
Head commit (8973aac)	3446 (+5)	3152 (+5)	91.47% (+0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#621)	10	10	100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

[coveralls]

Pull Request Test Coverage Report for Build 13826840492

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 12 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.01%) to 91.468%

---

Files with Coverage Reduction	New Missed Lines	%
nodes/for_loop.py	2	98.19%
node.py	10	91.36%

Totals
Change from base Build 13823447920:	0.01%
Covered Lines:	3152
Relevant Lines:	3446

---

💛 - Coveralls

[liamhuber]

There are a handful of other simple places where one needs to add extra conditions to see if the parent is running (i.e. if the parent is running, we want to go via the register_child_starting/finished/emitting, but if there is no parent or it's not running, we simply emit). This is a bit of legwork, but conceptually easy, just adding an extra handful of boolean clauses.

However, as is often the case, the flexible Workflow kills it all.

import pyiron_workflow as pwf

wf = pwf.Workflow("push_pull")
wf.n1 = pwf.standard_nodes.UserInput(0)
wf.n2 = pwf.standard_nodes.UserInput(wf.n1)
wf.n3 = pwf.standard_nodes.UserInput(wf.n2)

wf.n1.pull()
wf.n2.run()
print(wf.n3.outputs.user_input.value)
>>> NOT_DATA

This is because workflows configure their DAG execution on-the-fly at runtime. So wf.n2.run() runs n2 just fine, and even emits its signals, but...they have no receivers! This is a complete non-issue for macros, because they use the same Composite.set_run_signals_to_dag_execution to set up their signal flow at instantiation rather than runtime, so it's always available.

The way I got around this is by introducing Node.push, which is just a super hacky method for getting around this particular annoying edge case: is my parent a Workflow? Is it set to automate_execution? Then first self.parent.set_run_signals_to_dag_execution() and only then run. For children of macros, run and push are thus identical, while for children of Workflow, run may fail to propagate downstream under edge cases but push will always work (which may include not propagating if the signal flow is manually set to not do that).

So, the above would be fixed like

import pyiron_workflow as pwf

wf = pwf.Workflow("push_pull")
wf.n1 = pwf.standard_nodes.UserInput(0)
wf.n2 = pwf.standard_nodes.UserInput(wf.n1)
wf.n3 = pwf.standard_nodes.UserInput(wf.n2)

wf.n1.pull()
wf.n2.push()
print(wf.n3.outputs.user_input.value)
>>> 0

On the one hand, this is obviously a hack. It is ugly and I dislike it. On the other hand, I didn't want to add the boolean check/+flow configuration to each and every run call for performance reasons. I would very much appreciate feedback on which of these two others find to be the lesser of two evils -- or even better, if you have a clever idea (preferably including an implementation) for how to avoid the evil altogether.

[Tara-Lakshmipathy]

My perspective is a bit biased by the GUI. So, my opinion is that the ugly hack is totally fine because the GUI would only either be using (pull and push) or (pull and run) for the top level workflow. As far as I can tell, the main source of performance penalty in the GUI is from passing the JSON object between the view and the model. Compared to that, a performance hit by doing a push every time should not be noticable (at least I hope not). So, if you want to keep a low performance-penalty option available for the pure code side (i.e., run which does not trigger set_run_signals_to_dag_execution and boolean check of the parent), then I think that is totally fine.

Side note, I'm thinking of calling the pull button as simply "Run up to" and the push button as "Run from". Can't think of anything better right now. Suggestion are welcome and I will create an issue for the naming in the pyironflow repo once the features have been introduced.

[liamhuber]

Side note, I'm thinking of calling the pull button as simply "Run up to" and the push button as "Run from".

I think you'll find there's not enough room in the GUI for such verbose labels. In that way I would still prefer "push" and "pull", but I think your description is the perfect basis for a tooltip, the sort that appears when you hover over the button for a second.

[Tara-Lakshmipathy]

lgtm!