-
Notifications
You must be signed in to change notification settings - Fork 10
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
CLI: Add support for units to family show
#97
Conversation
370853b
to
c5d4fbc
Compare
Usage: $ aiida-pseudo family show -h
Usage: aiida-pseudo family show [OPTIONS] FAMILY
Show details of pseudo potential family.
Options:
-s, --stringency TEXT Stringency level for the recommended cutoffs.
-u, --unit TEXT Specify the energy unit of the cutoffs. Must be
recognized by the ``UnitRegistry`` of the ``pint``
library.
-r, --raw Display only raw query results, without any headers
or footers.
-h, --help Show this message and exit. Specifying no unit: $ aiida-pseudo family show SSSP/1.1/PBE/efficiency
Element Pseudo MD5 Wavefunction (Ry) Charge density (Ry)
--------- ------------------------------ -------------------------------- ------------------- ---------------------
Ag Ag_ONCV_PBE-1.0.oncvpsp.upf 94f47bd0669c641108e45594df92fabc 50.0 200.0
Al Al.pbe-n-kjpaw_psl.1.0.0.UPF cfc449ca30b5f3223ec38ddd88ac046d 30.0 240.0
Ar Ar_ONCV_PBE-1.1.oncvpsp.upf 46d28409cdd246843f76b7675277a949 60.0 240.0
As As.pbe-n-rrkjus_psl.0.2.UPF 767315de957beeeb34f87d97bf945c8f 35.0 280.0 Specifying hartree: $ aiida-pseudo family show SSSP/1.1/PBE/efficiency --unit hartree
Element Pseudo MD5 Wavefunction (hartree) Charge density (hartree)
--------- ------------------------------ -------------------------------- ------------------------ --------------------------
Ag Ag_ONCV_PBE-1.0.oncvpsp.upf 94f47bd0669c641108e45594df92fabc 25.0 100.0
Al Al.pbe-n-kjpaw_psl.1.0.0.UPF cfc449ca30b5f3223ec38ddd88ac046d 15.0 120.0
Ar Ar_ONCV_PBE-1.1.oncvpsp.upf 46d28409cdd246843f76b7675277a949 30.0 120.0
As As.pbe-n-rrkjus_psl.0.2.UPF 767315de957beeeb34f87d97bf945c8f 17.5 140.0 Specifying eV: $ aiida-pseudo family show SSSP/1.1/PBE/efficiency --unit eV
Element Pseudo MD5 Wavefunction (eV) Charge density (eV)
--------- ------------------------------ -------------------------------- ------------------- ---------------------
Ag Ag_ONCV_PBE-1.0.oncvpsp.upf 94f47bd0669c641108e45594df92fabc 680.3 2721.1
Al Al.pbe-n-kjpaw_psl.1.0.0.UPF cfc449ca30b5f3223ec38ddd88ac046d 408.2 3265.4
Ar Ar_ONCV_PBE-1.1.oncvpsp.upf 46d28409cdd246843f76b7675277a949 816.3 3265.4
As As.pbe-n-rrkjus_psl.0.2.UPF 767315de957beeeb34f87d97bf945c8f 476.2 3809.6 Specifying attojoules (because why not?): $ aiida-pseudo family show SSSP/1.1/PBE/efficiency --unit aJ
Element Pseudo MD5 Wavefunction (aJ) Charge density (aJ)
--------- ------------------------------ -------------------------------- ------------------- ---------------------
Ag Ag_ONCV_PBE-1.0.oncvpsp.upf 94f47bd0669c641108e45594df92fabc 109.0 436.0
Al Al.pbe-n-kjpaw_psl.1.0.0.UPF cfc449ca30b5f3223ec38ddd88ac046d 65.4 523.2
Ar Ar_ONCV_PBE-1.1.oncvpsp.upf 46d28409cdd246843f76b7675277a949 130.8 523.2
As As.pbe-n-rrkjus_psl.0.2.UPF 767315de957beeeb34f87d97bf945c8f 76.3 610.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mbercx , some suggestions
4d45e54
to
d899be8
Compare
Although we now support specifying units for the recommended cutoffs, the `family show` method had not been adapted for this feature. Here we add the `-u/--unit` option to the `family show` method, making sure to show the correct unit in the table header as well as adapting the values in the columns by supplying the unit to the `get_recommended_cutoffs` method. There is also some refactoring regarding the validation of the stringencies. The `RecommendedCutoffMixin.validate_stringency` method was not being used, so we adapt the code for several methods with the stringency input to rely on this validation method. This way we have a consistent and succinct error message in case the user requests a stringency that has not been configured.
d899be8
to
3577ffe
Compare
@sphuber this is ready for another round of review! I've still left some comments unresolved since there might still be some discussion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mbercx . Just a few remaining comments.
Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
d480f63
to
f8c9469
Compare
f8c9469
to
feaa2f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes #96
Although we now support specifying units for the recommended cutoffs,
the
family show
method had not been adapted for this feature.Here we add the
-u/--unit
option to thefamily show
method, makingsure to show the correct unit in the table header as well as adapting
the values in the columns by supplying the unit to the
get_recommended_cutoffs
method.There is also some refactoring regarding the validation of the
stringencies. The
RecommendedCutoffMixin.validate_stringency
methodwas not being used, so we adapt the code for several methods with the
stringency input to rely on this validation method. This way we have a
consistent and succinct error message in case the user requests a
stringency that has not been configured.