-
-
Notifications
You must be signed in to change notification settings - Fork 0
Take the cutoffs from the input when the pseudo family has none #127
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 10 commits
8c05443
b8939c2
b1ab107
419d6f0
e2660df
1a87738
354cd90
ecc12c9
cd50eb7
555b7b5
04dc886
5eab827
6ab63af
93444cb
a3035ef
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 |
|---|---|---|
|
|
@@ -115,6 +115,40 @@ def load_codes_for_task(workflow: WorkflowConfig) -> Codes: | |
| return codes | ||
|
|
||
|
|
||
| def pw_pseudo_overrides( | ||
| pseudo_family: str, | ||
| structure: orm.StructureData, | ||
| parameters: dict[str, Any], | ||
| ) -> dict[str, Any]: | ||
| """Return the ``pw`` override entries that pin a cutoff-less family's pseudos. | ||
|
|
||
| Empty for a family that publishes recommended cutoffs. Otherwise the | ||
| family's pseudos, which aiida-quantumespresso's protocol builder accepts | ||
| only when ``parameters['SYSTEM']`` carries both cutoffs — a pair | ||
| :func:`~koopmans.aiida.conversion.input_to_pw_parameters` completes from | ||
| ``ecutwfc`` alone. | ||
|
|
||
| Raises: | ||
| ValueError: If the family publishes no recommended cutoffs and the | ||
| input states none either. | ||
| """ | ||
| from koopmans.aiida.conversion import get_pseudos_from_family | ||
| from koopmans.aiida.setup.pseudos import pseudo_family_has_cutoffs | ||
|
|
||
| if pseudo_family_has_cutoffs(pseudo_family): | ||
| return {} | ||
|
|
||
| if "ecutwfc" not in parameters.get("SYSTEM", {}): | ||
| raise ValueError( | ||
| f"The pseudopotential family `{pseudo_family}` publishes no recommended " | ||
| "cutoffs, so they must come from the input file: set " | ||
| "`calculator_parameters.ecutwfc`. `ecutrho` follows at four times it " | ||
| "unless `calculator_parameters.pw.system.ecutrho` states otherwise." | ||
| ) | ||
|
|
||
| return {"pseudos": get_pseudos_from_family(pseudo_family, structure)} | ||
|
Owner
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 do we need to explicitly provide pseudos in this case? Why doesn't the
Collaborator
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. The family still supplies the pseudos — Handed a There is no way to tell it "use this family, take the cutoffs from me". But it has a second branch: given explicit Two things worth knowing:
Graded code-read, not reproduced: I have read upstream's two branches, and the tests pin that the pseudos arrive and the cutoffs are the input's. I have not separately reproduced the claim that the explicit- |
||
|
|
||
|
|
||
| def prepare_common_inputs( | ||
| koopmans_input: KoopmansInput, | ||
| override_keys: list[str], | ||
|
|
@@ -123,7 +157,9 @@ def prepare_common_inputs( | |
|
|
||
| Converts the koopmans input into a structure, ensures the pseudo family is | ||
| installed, and builds an overrides dict with a PW parameters entry for each | ||
| of the requested sub-workflow keys. | ||
| of the requested sub-workflow keys. A family publishing no recommended | ||
| cutoffs also has its pseudos pinned in that entry | ||
| (:func:`pw_pseudo_overrides`). | ||
|
|
||
| Args: | ||
| koopmans_input: The parsed koopmans input. | ||
|
|
@@ -141,6 +177,8 @@ def prepare_common_inputs( | |
| ensure_pseudo_family_installed(pseudo_family) | ||
|
|
||
| pw_overrides: dict[str, Any] = {"parameters": parameters} | ||
| pw_overrides.update(pw_pseudo_overrides(pseudo_family, structure, parameters)) | ||
|
|
||
| # The pw entry carries the pw.x parallelization directive: -npool rides | ||
| # settings.cmdline; ntasks rides metadata.options.resources — both survive | ||
| # get_builder_from_protocol's override merge (verified by eager build). | ||
|
|
||
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 is far too complex: Should just read: "ecutrho should be 4 x ecutwfc for nomr-conserving pseudos."
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.
Cut to the rule. It now reads:
Gone: the sentence explaining that the two converge together. Kept the value it should have been (180 Ry) and the key to drop, so the reader does not have to do the arithmetic or hunt for which key to edit.
04dc886.