-
Notifications
You must be signed in to change notification settings - Fork 24
Fix bugs in pseudopotentials panel #1504
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
Changes from all commits
e5536a5
b87b530
bffbbb0
5f0eaa4
6939c1c
b52b77b
644ba47
3ee1754
ec4bbe2
5d0807b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| import traitlets as tl | ||
| from aiida_pseudo.common.units import U | ||
| from aiida_pseudo.data.pseudo import UpfData | ||
|
|
||
| from aiida import orm | ||
| from aiida.common import exceptions | ||
|
|
@@ -46,16 +47,15 @@ class PseudosConfigurationSettingsModel( | |
| value_trait=tl.Unicode(allow_none=True), # pseudopotential node uuid | ||
| default_value={}, | ||
| ) | ||
| functional = tl.Unicode(allow_none=True) | ||
| functional = tl.Unicode(None, allow_none=True) | ||
| functional_options = tl.List( | ||
| trait=tl.Unicode(), | ||
| default_value=[ | ||
| "PBE", | ||
| "PBEsol", | ||
| ], | ||
| ) | ||
| functionals = tl.List(trait=tl.Unicode(allow_none=True)) | ||
| library = tl.Unicode(allow_none=True) | ||
| library = tl.Unicode(None, allow_none=True) | ||
| library_options = tl.List( | ||
| trait=tl.Unicode(), | ||
| default_value=[ | ||
|
|
@@ -67,7 +67,7 @@ class PseudosConfigurationSettingsModel( | |
| "PseudoDojo stringent (FR)", | ||
| ], | ||
| ) | ||
| family = tl.Unicode(allow_none=True) | ||
| family = tl.Unicode(None, allow_none=True) | ||
| family_header = tl.Unicode(allow_none=True) | ||
| cutoffs = tl.List( | ||
| trait=tl.List(tl.Float()), # [[ecutwfc values], [ecutrho values]] | ||
|
|
@@ -112,13 +112,12 @@ def update(self, specific=""): | |
| family = self.family | ||
| self.update_family_parameters() | ||
| # When the app starts, the family is not yet set. `update_family_parameters` | ||
| # will set the family, which will set the functionals and dictionary. | ||
| # will set the family, which will set the dictionary. | ||
| # However, when the structure is changed, the family may already be set to | ||
| # the default, in which case, the functionals and dictionary will not be | ||
| # the default, in which case, the dictionary will not be | ||
| # updated. Therefore, we need to force the update. | ||
| if specific == "structure" and self.family == family: | ||
| self.update_dictionary() | ||
| self.update_functionals() | ||
|
|
||
| def update_family_parameters(self): | ||
| if self.locked: | ||
|
|
@@ -149,17 +148,8 @@ def update_family_parameters(self): | |
| self.functional = self._defaults["functional"] | ||
| self.library = self._defaults["library"] | ||
|
|
||
| def update_functionals(self): | ||
| if self.locked or not (self.functional and self.family): | ||
| return | ||
| self.functionals = ( | ||
| [self.functional] * len(self.input_structure.kinds) | ||
| if self.has_structure | ||
| else [] | ||
| ) | ||
|
Comment on lines
-152
to
-159
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need for |
||
|
|
||
| def update_family(self): | ||
| if self.locked or not (self.library and self.functional): | ||
| if not (self.library and self.functional) or self.locked: | ||
| return | ||
|
|
||
| parts = self.library.split() | ||
|
|
@@ -224,7 +214,7 @@ def update_dictionary(self): | |
| f"Failed to fetch pseudo family using the '{self.family}' string" | ||
| ) from err | ||
|
|
||
| pseudos = {} | ||
| pseudos: dict[str, UpfData | None] = {} | ||
| for kind in self.input_structure.kinds: | ||
| # If the kind is not in the family, we set it to None. | ||
| # This will block the app and notify the user of the missing pseudo. | ||
|
|
@@ -246,7 +236,7 @@ def update_dictionary(self): | |
| self.dictionary = self._get_default_dictionary() | ||
|
|
||
| def update_cutoffs(self): | ||
| if self.locked or not self.dictionary: | ||
| if not self.dictionary or self.locked: | ||
| return | ||
|
|
||
| kinds = self.input_structure.kinds if self.has_structure else [] | ||
|
|
@@ -313,7 +303,7 @@ def update_cutoffs(self): | |
| self.cutoffs = self._get_default_cutoffs() | ||
|
|
||
| def update_library_options(self): | ||
| if self.locked or not self.has_structure: | ||
| if not self.has_structure or self.locked: | ||
| return | ||
|
|
||
| relativistic_options = [ | ||
|
|
@@ -338,6 +328,36 @@ def update_library_options(self): | |
|
|
||
| self.update_family_parameters() | ||
|
|
||
| def get_cutoffs_by_index(self, index: int) -> list[float]: | ||
| return ( | ||
| [self.cutoffs[0][index], self.cutoffs[1][index]] | ||
| if len(self.cutoffs[0]) > index | ||
| else [0.0, 0.0] | ||
| ) | ||
|
Comment on lines
+331
to
+336
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cleaner to have this helper here than in the settings panel. |
||
|
|
||
| def update_functional(self): | ||
| pseudos: list[UpfData] = [] | ||
| for kind_name, uuid in self.dictionary.items(): | ||
| kind = self.input_structure.get_kind(kind_name) | ||
| try: | ||
| assert uuid is not None | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looking at some older code (see the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, there are missing Psedos for SOC for lanthanides... yes |
||
| pseudo = orm.load_node(uuid) | ||
| pseudos.append(pseudo) | ||
| except AssertionError: | ||
| print( | ||
| f"The selected pseudopotential family does not contain a pseudopotential for {kind.symbol}. Consider changing the family or uploading a custom pseudopotential." | ||
| ) | ||
| continue | ||
| except exceptions.NotExistent: | ||
| print( | ||
| f"Pseudopotential with UUID {uuid} does not exist for {kind.symbol}." | ||
| ) | ||
| continue | ||
| functional_set = {pp.base.extras.get("functional", None) for pp in pseudos} | ||
| functional = functional_set.pop() if len(functional_set) == 1 else None | ||
| self._defaults["functional"] = functional | ||
| self.functional = self._get_default("functional") | ||
|
Comment on lines
+338
to
+359
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is called on change of pseudos in the dictionary. It collects each pseudo's functional (from extras), and sets the |
||
|
|
||
| def reset(self): | ||
| with self.hold_trait_notifications(): | ||
| self.dictionary = self._get_default("dictionary") | ||
|
|
@@ -386,10 +406,10 @@ def _get_default_cutoffs(self): | |
| return deepcopy(self._defaults["cutoffs"]) | ||
|
|
||
| def _check_blockers(self): | ||
| if not (self.dictionary and self.functionals): | ||
| if not (self.has_structure and self.dictionary): | ||
| return | ||
|
|
||
| pseudos = [] | ||
| pseudos: list[UpfData] = [] | ||
| for kind_name, uuid in self.dictionary.items(): | ||
| kind = self.input_structure.get_kind(kind_name) | ||
| try: | ||
|
|
@@ -403,7 +423,7 @@ def _check_blockers(self): | |
| yield f"Pseudopotential with UUID {uuid} does not exist for {kind.symbol}." | ||
| return | ||
|
|
||
| functional_set = set(self.functionals) | ||
| functional_set = {pp.base.extras.get("functional", None) for pp in pseudos} | ||
| if len(functional_set) != 1: | ||
| yield "All pseudopotentials must have the same exchange-correlation (XC) functional." | ||
| elif self.functional and self.functional not in functional_set: | ||
|
|
@@ -413,3 +433,9 @@ def _check_blockers(self): | |
| if self.spin_orbit == "soc": | ||
| if relativistic_set != {"full"}: | ||
| yield "For spin-orbit coupling (SOC) calculations, all pseudopotentials must be fully relativistic." | ||
|
|
||
| if self.ecutwfc == 0.0: | ||
|
AndresOrtegaGuerrero marked this conversation as resolved.
|
||
| yield "The cutoff energy for wavefunctions (ecutwfc) cannot be zero." | ||
|
|
||
| if self.ecutrho == 0.0: | ||
| yield "The cutoff energy for charge density (ecutrho) cannot be zero." | ||
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.
This part I suspect can be simplified, but immediate efforts failed. Keeping as is for now.