Skip to content
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

[minor] Replace while with a macro pattern #364

Merged
merged 7 commits into from
Jun 12, 2024
Merged

[minor] Replace while with a macro pattern #364

merged 7 commits into from
Jun 12, 2024

Conversation

liamhuber
Copy link
Member

The current while-loop metanode is (a) not a nice UI at all, and (b) not pickleable (or even easy to read/understand). I set out to replace it with something like the For class and for_node constructor for for-loops, but didn't find a way of breaking things out well enough to get a clean UI.

In lieu of replacing it with a new tool, here I just remove it entirely and replace it with a raw example (in both the deepdive and the integration tests) how you can make a cyclic subgraph directly as a macro. There is still some "universal" aspects to this that I'd love to eventually abstract out to a @as_while_loop decorator or something, but in the meantime I found sticking to a plain macro makes it much cleaner to handle setting up both the logic and the IO.

As long as body functionality and condition functionality are in turn nicely abstracted into their own nodes/macros, the example below is extremely generic. Note also that since the same nodes get executed multiple times, it also shows how the (new to this PR) AppendToList node can be used to track the parts of the history of the loop that is important to you.

from pyiron_workflow import Workflow

@Workflow.wrap.as_macro_node("greater")
def AddWhileLessThan(self, a, b, cap):
    """
    Add :param:`b` to :param:`a` while the sum is less than or equal to :param:`cap`.

    A simple but complete demonstrator for how to construct cyclic flows, including
    logging key outputs during the loop.
    """
    # Bespoke logic
    self.body = Workflow.create.standard.Add(obj=a, other=b)
    self.body.inputs.obj = self.body.outputs.add  # Higher priority connection
    # The output is NOT_DATA on the first pass and `a` gets used,
    # But after that the node will find and use its own output
    self.condition = Workflow.create.standard.LessThan(self.body, cap)

    # Universal logic
    self.switch = Workflow.create.standard.If(condition=self.condition)

    self.starting_nodes = [self.body]
    self.body >> self.condition >> self.switch
    self.switch.signals.output.true >> self.body

    # Bespoke logging
    self.history = Workflow.create.standard.AppendToList()
    self.history.inputs.existing = self.history
    self.history.inputs.new_element = self.body
    self.body >> self.history

    # Returns are pretty universal for single-value body nodes,
    # assuming a log of the history is not desired as output,
    # but in general return values are also bespoke
    return self.body

awlt = AddWhileLessThan()
out = awlt(0, 2, 5)

print(out)
>>> {'greater': 6}

print(awlt.history.outputs.list.value)
>>> [2, 4, 6]

awlt.draw(size=(10, 10))

while_loop

This is pickleable and clear to write and read. I don't have a convenience wrapper yet to get rid of the "universal" stuff though.
It was janky, the UI was not even particularly nice, and it wouldn't pickle. Just axe it.
Instead of the old while metanode
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_workflow/fix_while

@liamhuber liamhuber changed the title Fix while [minor] Replace while Jun 12, 2024
@liamhuber liamhuber changed the title [minor] Replace while [minor] Replace while with a macro pattern Jun 12, 2024
Copy link

codacy-production bot commented Jun 12, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
-0.12% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (612b4ee) 3430 3176 92.59%
Head commit (2a8fa88) 3415 (-15) 3158 (-18) 92.47% (-0.12%)

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 (#364) 6 6 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


🚀 Don’t miss a bit, follow what’s new on Codacy.

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

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9488435797

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 21 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.1%) to 92.477%

Files with Coverage Reduction New Missed Lines %
create.py 9 92.16%
nodes/standard.py 12 91.72%
Totals Coverage Status
Change from base Build 9473872395: -0.1%
Covered Lines: 3159
Relevant Lines: 3416

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9488481429

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 21 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.1%) to 92.474%

Files with Coverage Reduction New Missed Lines %
create.py 9 92.16%
nodes/standard.py 12 91.67%
Totals Coverage Status
Change from base Build 9473872395: -0.1%
Covered Lines: 3158
Relevant Lines: 3415

💛 - Coveralls

@liamhuber liamhuber merged commit 7f48546 into main Jun 12, 2024
16 of 17 checks passed
@liamhuber liamhuber deleted the fix_while branch June 12, 2024 19:31
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.

None yet

2 participants