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

Protocols: Add usage of metadata/paral. overrides #652

Merged

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented Feb 18, 2021

Fixes #651

The metadata and parallelization inputs of the PwCalculation,
stored in the pw namespace of the PwBaseWorkChain, are not
(properly) used in the get_builder_from_protocol() method. Here we
make the following changes to correct this issue:

  • Move the metadata inputs under pw in the base.yaml file.
  • Correctly obtain the metadata from the inputs in the
    get_builder_from_protocol() method of the PwBaseWorkChain.
  • Check if there is parallelization input in the pw namespace of the
    inputs, and if so add it to the builder in the
    get_builder_from_protocol() method of the PwBaseWorkChain.

@mbercx mbercx force-pushed the fix/651/metadata-parallel-override branch from 002da7a to ffc373f Compare February 18, 2021 15:36
@mbercx mbercx force-pushed the fix/651/metadata-parallel-override branch from ffc373f to 1a6278a Compare May 6, 2021 09:45
@mbercx
Copy link
Member Author

mbercx commented May 6, 2021

@sphuber although I think the comment in your review is valid, I will open a separate PR to deal with the possibility of overrides being e.g. Dict nodes for all inputs constructed in the get_builder_from_protocol methods of all work chains. If you have no further comments, perhaps this PR can be merged so it can be added to the next patch release?

Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

Ok to merge as is @mbercx . Would it be possible to add a quick test that the metadata and parallelization are now respected by the get_builder_from _protocol?

mbercx added 3 commits May 6, 2021 20:27
The `metadata` and `parallelization` inputs of the `PwCalculation`,
stored in the `pw` namespace of the `PwBaseWorkChain`, are not
(properly) used in the `get_builder_from_protocol()` method. Here we
make the following changes to correct this issue:

* Move the `metadata` inputs under `pw` in the base.yaml file.
* Correctly obtain the `metadata` from the inputs in the
`get_builder_from_protocol()` method of the `PwBaseWorkChain`.
* Check if there is `parallelization` input in the `pw` namespace of the
inputs, and if so add it to the builder in the
`get_builder_from_protocol()` method of the `PwBaseWorkChain`.
@mbercx mbercx force-pushed the fix/651/metadata-parallel-override branch from 1a6278a to 1bd67b4 Compare May 6, 2021 18:27
@mbercx
Copy link
Member Author

mbercx commented May 6, 2021

2 addition tests, coming right up!

@mbercx mbercx requested a review from sphuber May 6, 2021 18:27
Copy link
Contributor

@sphuber sphuber left a comment

Choose a reason for hiding this comment

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

@sphuber sphuber merged commit bedbd57 into aiidateam:develop May 6, 2021
@mbercx mbercx deleted the fix/651/metadata-parallel-override branch May 6, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Protocols: metadata and parallelization overrides not used
2 participants