-
Notifications
You must be signed in to change notification settings - Fork 192
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
[WIP]: expose inputs and port namespacing #1070
Closed
sphuber
wants to merge
42
commits into
aiidateam:workflows
from
sphuber:fix_660_expose_workchain_inputs
Closed
[WIP]: expose inputs and port namespacing #1070
sphuber
wants to merge
42
commits into
aiidateam:workflows
from
sphuber:fix_660_expose_workchain_inputs
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
This change was implemented to accommodate the new functionality that allows a Process to expose the inputs of another Process. The goal for WorkChain design should be to write well defined abstract components that can then be composed together by wrapping workchains. To prevent to having to manually copy over the input specification of a sub workchain we provide the 'expose_inputs' method that will do this automatically. To allow for inputs to be separated, we change the inputs of the ProcessSpec to be a PortNamespace, which allows for arbitrary nesting of Ports.
This option allows the exposing of only a particular subset of inputs of a Process.
A common use case is for a wrapping Process to run multiple instances of another sub Process and therefore wants to expose its inputs. However, the required inputs for the sub Process instances may differ for some inputs, while others can be shared. To accomplish this one can expose the inputs twice in separate namespaces and expose the shared inputs on the top level namespace. To simplify the specification of which inputs to expose where, we introduce the 'include' parameter for the ProcessSpec.expose_inputs method. To simply the gathering of inputs from the parsed inputs for a given sub process over multiple namespaces, we adapt the behaviour of the exposed_inputs methods to traverse down the namespace tree, collecting all inputs that are common to the process in question. This agglomerating behavior can be prevented by passing the argument agglomerate=False
Change the default for 'include' to None.
Add test where the same dict is passed for two namespaces
Fix typo in agglomerate tests
…aiida_core into fix_660_expose_workchain_inputs
So that it can be closed automatically when the user is done using it.
It's not all stored in rmq.py (connection paramteres and prefix).
The Process class had custom implemented functions to encode and decode inputs for saving and loading its instance state. Swap them for the general serialize_data and deserialize_data
The second one properly takes the USE_TZ setting into account
Now both the exchange and task queue use the prefix properly.
The LaunchProcessAction and ExecuteProcessAction will receive payloads with potentially unstored nodes, which before being sent over RMQ, will have to be stored and serialized. On the receiving end the ProcessLauncher needs to deserialize the inputs dictionary before passing it to the process class constructor
This will run the new daemon in the current interpreter, logging all messages to stdout
Removed dead commented out code and an unused function argument
They have been implemented in plum
…_expose_workchain_inputs
This was introduced during a big merge conflict of the daemon's execmanager some time ago
The ProcessSpec class in plum has been changed quite a lot to support generic port namespacing in the inputs and outputs. This requires small changes in the interface
The only calculation that was being tested against the JobProcess was the TemplatereplacerCalculation, which doesn't have a use method with an additional parameter. As such the InputGroup functionality was not being tested
…pace The InputGroup functionality has been made obsolete by an improved and generalized implementation of port namespacing through the PortNamespace class. To create the necessary port namespace for a use method containing an additional parameter, we can now use ProcessSpec.input_namespace()
This was a horribly messy branch, with many commits of code that ended up being migrated to plum. I have opted to create a clean branch and open a new PR #1084 |
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.
Fixes #660
The final implementation is still in the works in plum