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

CLI: Add support for units to family show #97

Merged
merged 3 commits into from
May 12, 2021

Conversation

mbercx
Copy link
Member

@mbercx mbercx commented May 9, 2021

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 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.

@mbercx mbercx requested a review from sphuber May 9, 2021 07:39
@mbercx mbercx force-pushed the fix/96/family-show-units branch from 370853b to c5d4fbc Compare May 9, 2021 07:40
@mbercx
Copy link
Member Author

mbercx commented May 9, 2021

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

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.

Thanks @mbercx , some suggestions

aiida_pseudo/cli/family.py Outdated Show resolved Hide resolved
aiida_pseudo/cli/family.py Outdated Show resolved Hide resolved
aiida_pseudo/groups/mixins/cutoffs.py Outdated Show resolved Hide resolved
aiida_pseudo/groups/mixins/cutoffs.py Outdated Show resolved Hide resolved
aiida_pseudo/groups/mixins/cutoffs.py Show resolved Hide resolved
tests/cli/test_family.py Outdated Show resolved Hide resolved
@mbercx mbercx force-pushed the fix/96/family-show-units branch 2 times, most recently from 4d45e54 to d899be8 Compare May 9, 2021 13:55
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.
@mbercx mbercx force-pushed the fix/96/family-show-units branch from d899be8 to 3577ffe Compare May 9, 2021 13:56
@mbercx mbercx requested a review from sphuber May 9, 2021 13:58
@mbercx
Copy link
Member Author

mbercx commented May 9, 2021

@sphuber this is ready for another round of review! I've still left some comments unresolved since there might still be some discussion.

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.

Thanks @mbercx . Just a few remaining comments.

aiida_pseudo/cli/params/types.py Outdated Show resolved Hide resolved
aiida_pseudo/cli/params/types.py Outdated Show resolved Hide resolved
aiida_pseudo/cli/params/types.py Outdated Show resolved Hide resolved
aiida_pseudo/groups/mixins/cutoffs.py Outdated Show resolved Hide resolved
aiida_pseudo/groups/mixins/cutoffs.py Show resolved Hide resolved
Co-authored-by: Sebastiaan Huber <mail@sphuber.net>
@mbercx mbercx force-pushed the fix/96/family-show-units branch from d480f63 to f8c9469 Compare May 11, 2021 21:54
@mbercx mbercx force-pushed the fix/96/family-show-units branch from f8c9469 to feaa2f2 Compare May 11, 2021 21:57
@mbercx mbercx requested a review from sphuber May 11, 2021 22:15
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 bb520e3 into aiidateam:master May 12, 2021
@mbercx mbercx deleted the fix/96/family-show-units branch May 12, 2021 09:52
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.

CLI: Add support for units to family show
2 participants