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

install family: allow installation of sssp and pseudo-dojo #92

Closed
mbercx opened this issue May 8, 2021 · 3 comments · Fixed by #135
Closed

install family: allow installation of sssp and pseudo-dojo #92

mbercx opened this issue May 8, 2021 · 3 comments · Fixed by #135

Comments

@mbercx
Copy link
Member

mbercx commented May 8, 2021

In #80, we blocked pseudo.family.sssp and pseudo.family.pseudo_dojo for the -F, --family-type option. The rationale was that they have their own dedicated install commands, but these don't allow the user to install from an archive, so our workaround for installing these families in case of connection issues is faulty. Note that the user could also just install these pseudos as a CutoffsPseudoPotentialFamily, but this would mean they are no longer a pseudo family of the same class as the automated install, which can be an issue if a plugin that uses aiida-pseudo requires the pseudopotentials to be install as a specific class. Moreover, it would be more confusing to the user who has to use this workaround.

The solution is to simply no longer block these options, so the user can install the downloaded archives as a proper SsspFamily or PseudoDojoFamily.

Thanks to @atztogo for reporting!

@mbercx
Copy link
Member Author

mbercx commented May 8, 2021

An alternative solution would be to add an --from-archive option to the aiida-pseudo install sssp/pseudo-dojo commands. Or maybe we could even do both? I don't see a very strong motivation for blocking the manual installation of pseudo.family.sssp and pseudo.family.pseudo_dojo pseudopotential families via install family, maybe you disagree @sphuber?

@mbercx
Copy link
Member Author

mbercx commented May 8, 2021

One motivation might be that we want to make sure that the labels for the SSSP/pseudo-dojo are correct, since the get_builder_from_protocol() method of e.g. aiida-quantumespresso sets defaults that rely on this label being correct. Then we'd have to add the label to the archive, and only allow the installation of SSSP from archive via aiida-pseudo install sssp --from-archive. Not sure if that is worth it though, I'm still leaning to just allowing manual installs of SSSP/pseudo-dojo with install family and separating manual and automated installs entirely. If someone uses the --from-archive option, they might mistakenly believe that the cutoffs are set as well.

@sphuber
Copy link
Contributor

sphuber commented May 8, 2021

The original rationale was that I wanted to give "some" security that if an SsspFamily was installed in the system, it represented an actual official SSSP and not some random collection of pseudos and cutoffs. Note that we could never fully prevent this, because in the shell you can instantiate whatever, but I thought to not make it easy through the CLI, not thinking there would ever be a problem with the automated install. Maybe it is causing more problems than it is doing good though, so maybe we can indeed lift the constraint.

Question then becomes indeed whether we reserve install family to install those family types, or if we update install sssp and install pseudo-dojo to take archive and metadata file from disk, as you say. Note that we could also just add an option to specify the metadata so you can still install a complete family. But maybe it is indeed better to separate them. The problem with this approach is though that the metadata files of the SSSP and Pseudo Dojo have their own particular format that is somewhat baked into the automated commands, but wouldn't be directly pluggable into family cutoffs set, as you reported in #93 . I am not sure what to do here. I think that we could make the validation a bit looser and for example ignore extraneous keys, which would solve this issue for the SSSP metadata, but there is not guarantee this works for Pseudo Dojo. And I am not sure if we should start to add all sorts of custom logic for those in the generic command when there is the family specific command that already contains that logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment