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

Implement the usage of plum PortNamespaces #1099

Merged

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Feb 1, 2018

Fixes #1096

This implements the changes necessary to leverage the recently added PortNamespace in plum.
The inputs and outputs of a ProcessSpec are now port namespaces, which means that they can be arbitrarily nested. This also allowed us to get rid of the input_group method and replace it by a proper PortNamespace which can now be used for JobProcesses that have use methods with additional parameters.

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()
Methods like dynamic_input and dynamic_output have been deprecated
in the latest plum due to the introduction of PortNamespaces.
To make the input or output ports of a ProcessSpec dynamic or not
one has to now go directly through the relevant port namespace.
@sphuber sphuber added the type/accepted feature approved feature request label Feb 1, 2018
@sphuber sphuber added this to the v0.12.0 milestone Feb 1, 2018
@sphuber sphuber requested a review from muhrin February 1, 2018 17:30
if port.non_db:
continue

items.append((prefixed_key, value))
Copy link
Contributor

Choose a reason for hiding this comment

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

This will be called a second time if the port does not exist in the spec (i.e. is dynamic).

Changing this brings a new version of kiwipy which required a small
change in the RmqConnector API to close it
When setting up the database record for a Process, the dictionary of
inputs, which may be nested, has to be flattened, skipping any inputs
whose corresponding port is marked as non_db, i.e. an input that should
not be stored as a node in the database. The link labels will be created
by concatenating the namespaces with underscores.

Also make sure that the port types of the plum ProcessSpec are overridden
with the versions of aiida core, which have the WithNonDb mixin.
…nd thus immutable

Plum is currently not recursing down nested input dictionaries when creating
the parsed input data structure of a Process, which means that only the top
level inputs (reachable through self.inputs from within the Process) are
wrapped in an AttributesFrozendict and therefore immutable. Any inputs in an
input namespace will be bare dictionaries and therefore mutable. This requires
a fix in plum, but two tests have been added that test this behavior. The one
with the namespaced inputs currently fails.
@sphuber
Copy link
Contributor Author

sphuber commented Feb 3, 2018

This PR will also address the issues raised in #919 . In commit 523d2eb I added the tests written by @giovannipizzi that address the problem and slightly modified them to adapt them to the workflows branch. With the implementation of PortNamespaces in plum and the most recent bug fixes, this problem should now be fixed as indicated by the passing tests

@muhrin muhrin merged commit ba9f9fa into aiidateam:workflows Feb 5, 2018
@muhrin muhrin added this to Done in Engine via automation Feb 5, 2018
@sphuber sphuber deleted the fix_1096_process_spec_port_namespaces branch February 12, 2018 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/accepted feature approved feature request
Projects
No open projects
Engine
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants