BaseRestartWorkChain: add the get_outputs hook
#5618
Merged
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
mbercx
previously approved these changes
Aug 10, 2022
There was a problem hiding this comment.
Looks good @sphuber! Field tested in the context of aiidateam/aiida-quantumespresso#818, and it serves this use case perfectly. But maybe we should still add a bit more explanation on why there is a separate method to get the outputs?
| @@ -295,6 +295,10 @@ def inspect_process(self) -> Optional['ExitCode']: # pylint: disable=too-many-b | |||
|
|
|||
| return None | |||
|
|
|||
| def get_outputs(self, node) -> Mapping[str, orm.Node]: | |||
| """Return a mapping of the outputs that should be attached as outputs to the work chain.""" | |||
There was a problem hiding this comment.
Should we still add a sentence of two explaining that this method can be overridden in case the BaseRestartWorkChain needs to adjust the outputs of the underlying CalcJob somehow?
sphuber
force-pushed
the
feature/5617/base-restart-get-results
branch
from
ea0e533
to
a659314
Compare
|
Thanks @mbercx . I have updated the docstring and added some documentation |
The `BaseRestartWorkChain`, in the `results` outline step, will take the outputs of the last completed `CalcJob` and attach them to the work chain itself. There are use cases where the implementation may want to slightly adapt the outputs before attaching them. By abstracting the code that fetches the outputs to a separate method `get_outputs`, it provides a hook for implementations to customize this behavior.
sphuber
force-pushed
the
feature/5617/base-restart-get-results
branch
from
a659314
to
92e874f
Compare
mbercx
approved these changes
Aug 11, 2022
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 #5617
The
BaseRestartWorkChain, in theresultsoutline step, will take theoutputs of the last completed
CalcJoband attach them to the workchain itself. There are use cases where the implementation may want to
slightly adapt the outputs before attaching them. By abstracting the
code that fetches the outputs to a separate method
get_outputs, itprovides a hook for implementations to customize this behavior.