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

PwBaseWorkChain: deprecate the pseudo_family input #603

Merged
merged 1 commit into from
Oct 29, 2020

Conversation

sphuber
Copy link
Contributor

@sphuber sphuber commented Oct 28, 2020

Fixes #601

The use of a pseudo_family, which has to be passed in as a Str node
to represent the name of a Group, is not robust with respect to the
provenance. Since groups are mutable, rerunning a workchain that used
the pseudo_family to determine what pseudos to run, can lead to a
rerun using different pseudos. This input exists for historical reasons
because it made the usage of the workflows easier for users, since they
wouldn't have to build the mapping of pseudos themselves, but with the
advent of aiida-pseudo and the input generators, this is no longer an
issue and it is preferable to directly pass the pseudos into the
pseudos input namespace.

@sphuber sphuber requested a review from mbercx October 28, 2020 13:38
@@ -159,6 +159,11 @@ def validate_pseudos(self):
pseudos = self.ctx.inputs.get('pseudos', None)
pseudo_family = self.inputs.get('pseudo_family', None)

if pseudo_family:
import warnings
from aiida.common.warnings import AiidaDeprecationWarning
Copy link
Member

Choose a reason for hiding this comment

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

Similar to our discussion in #569, shouldn't we create a validator instead of raising a warning, which will just wind up in the daemon logs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

very good point

The use of a `pseudo_family`, which has to be passed in as a `Str` node
to represent the name of a `Group`, is not robust with respect to the
provenance. Since groups are mutable, rerunning a workchain that used
the `pseudo_family` to determine what pseudos to run, can lead to a
rerun using different pseudos. This input exists for historical reasons
because it made the usage of the workflows easier for users, since they
wouldn't have to build the mapping of pseudos themselves, but with the
advent of `aiida-pseudo` and the input generators, this is no longer an
issue and it is preferable to directly pass the pseudos into the
`pseudos` input namespace.

Also deprecate the functions in `utils.pseudopotential`, one of which is
not even used currently, and the other two which will be obsolete as
they are replaced by method directly integrated into the pseudo family
groups.
@sphuber sphuber force-pushed the fix/601/pw-base-deprecate-pseudo-family branch from e2d5605 to cf79742 Compare October 29, 2020 16:42
Copy link
Member

@mbercx mbercx left a comment

Choose a reason for hiding this comment

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

Thanks @sphuber! LGTM! 🚀

@sphuber sphuber merged commit 41e5423 into develop Oct 29, 2020
@sphuber sphuber deleted the fix/601/pw-base-deprecate-pseudo-family branch October 29, 2020 16:57
sphuber added a commit that referenced this pull request Nov 17, 2020
The use of a `pseudo_family`, which has to be passed in as a `Str` node
to represent the name of a `Group`, is not robust with respect to the
provenance. Since groups are mutable, rerunning a workchain that used
the `pseudo_family` to determine what pseudos to run, can lead to a
rerun using different pseudos. This input exists for historical reasons
because it made the usage of the workflows easier for users, since they
wouldn't have to build the mapping of pseudos themselves, but with the
advent of `aiida-pseudo` and the input generators, this is no longer an
issue and it is preferable to directly pass the pseudos into the
`pseudos` input namespace.

Also deprecate the functions in `utils.pseudopotential`, one of which is
not even used currently, and the other two which will be obsolete as
they are replaced by method directly integrated into the pseudo family
groups.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deprecate use of pseudo_family in PwBaseWorkChain
2 participants