generated from pyiron/pyiron_module_template
-
Notifications
You must be signed in to change notification settings - Fork 2
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
Callbacks as strings #174
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # pyiron_workflow/channels.py
And clarify in the docstring that it must have no arguments
Since we've guaranteed it lives on the owning node, this is safe now
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.
samwaseda
approved these changes
Jan 29, 2024
There was a problem hiding this 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)
samwaseda
reviewed
Jan 30, 2024
And make it more efficient with early stopping Co-authored-by: Sam Dareska <[email protected]>
🐛 stop running
Copy and update __dict__
Composites pass self instead of children and starting nodes
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 onFurther, 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.