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

Callbacks as strings #174

Merged
merged 20 commits into from
Jan 30, 2024
Merged

Callbacks as strings #174

merged 20 commits into from
Jan 30, 2024

Conversation

liamhuber
Copy link
Member

This is stacked on top of #160, but looks more complicated than it is because it also merged in #173. Once #173 is merged, just the 4 commits from 23 Jan will show here.

Basically we add the requirement that callback functions stored on InputSignal need to be methods of the node that owns them. In practice this has always been the case, so there is no real change, and since you can put whatever method you want on

Further, the requirement that callbacks be arg-free was already stated, but now we explicitly test for it and fail hard if it's violated.

Since the callback is now guaranteed to be a method of the owning InputSignal.node attribute, we can safely store its name in our state (a mere string) instead of storing the actual function object. This will make storage easier later.

Copy link

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

@liamhuber liamhuber added the format_black trigger the Black formatting bot label Jan 24, 2024
@liamhuber liamhuber marked this pull request as ready for review January 24, 2024 17:28
liamhuber and others added 8 commits January 24, 2024 11:21
Actually use the state we create!
Instead of just the children. In the case of macros, which are not parent-most objects, we need to be sure to restore any connections the local object had when parsing the remote result
We parse the whole composite now, not just its children
Sometimes we mess with stuff in __getstate__, e.g. to avoid reflexive relationships between nodes and channels, but getting the state should never modify the current state, so always make a fresh dictionary. Similarly, the returned state might possibly not have all the content our current state does, so use update instead of overwriting.
In the case of running on an executor
By setting `running=False` on the local instance _before_ updating the state, we were then re-writing `running=True` when updating the state. Update the flag on the received instance as well, and add a test so the same issue doesn't crop up again.
Copy link
Member

@samwaseda samwaseda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me (except for just one minor comment)

liamhuber and others added 4 commits January 30, 2024 07:58
And make it more efficient with early stopping

Co-authored-by: Sam Dareska <[email protected]>
Composites pass self instead of children and starting nodes
@liamhuber liamhuber merged commit 6c401ab into simple_storage Jan 30, 2024
4 of 11 checks passed
@liamhuber liamhuber deleted the callbacks_as_strings branch January 30, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black trigger the Black formatting bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants