diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7328637..3f5631b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -44,7 +44,7 @@ jobs: git clone --depth 1 --branch patched https://github.com/elinscott/node-graph ../node-graph git clone --depth 1 --branch patched https://github.com/elinscott/plumpy ../plumpy git clone --depth 1 --branch patched https://github.com/elinscott/aiida-wannier90-workflows ../aiida-wannier90-workflows - git init -q ../aiida-quantumespresso && git -C ../aiida-quantumespresso fetch --depth 1 https://github.com/aiidateam/aiida-quantumespresso 0a15b8ac63d35f1422ba0b2184f420c177bd2d70 && git -C ../aiida-quantumespresso checkout -q FETCH_HEAD + git clone --depth 1 --branch patched https://github.com/elinscott/aiida-quantumespresso ../aiida-quantumespresso git clone --depth 1 https://github.com/elinscott/aiida-wannierjl ../aiida-wannierjl git clone --depth 1 https://github.com/elinscott/pydantic-espresso ../pydantic_espresso git init -q ../aiida-core && git -C ../aiida-core fetch --depth 1 https://github.com/aiidateam/aiida-core 4c81e9d64ac561325b91c7e6a18ea2cf792e0609 && git -C ../aiida-core checkout -q FETCH_HEAD @@ -86,7 +86,7 @@ jobs: git clone --depth 1 --branch patched https://github.com/elinscott/node-graph ../node-graph git clone --depth 1 --branch patched https://github.com/elinscott/plumpy ../plumpy git clone --depth 1 --branch patched https://github.com/elinscott/aiida-wannier90-workflows ../aiida-wannier90-workflows - git init -q ../aiida-quantumespresso && git -C ../aiida-quantumespresso fetch --depth 1 https://github.com/aiidateam/aiida-quantumespresso 0a15b8ac63d35f1422ba0b2184f420c177bd2d70 && git -C ../aiida-quantumespresso checkout -q FETCH_HEAD + git clone --depth 1 --branch patched https://github.com/elinscott/aiida-quantumespresso ../aiida-quantumespresso git clone --depth 1 https://github.com/elinscott/aiida-wannierjl ../aiida-wannierjl git clone --depth 1 https://github.com/elinscott/pydantic-espresso ../pydantic_espresso git init -q ../aiida-core && git -C ../aiida-core fetch --depth 1 https://github.com/aiidateam/aiida-core 4c81e9d64ac561325b91c7e6a18ea2cf792e0609 && git -C ../aiida-core checkout -q FETCH_HEAD @@ -136,7 +136,7 @@ jobs: git clone --depth 1 --branch patched https://github.com/elinscott/node-graph ../node-graph git clone --depth 1 --branch patched https://github.com/elinscott/plumpy ../plumpy git clone --depth 1 --branch patched https://github.com/elinscott/aiida-wannier90-workflows ../aiida-wannier90-workflows - git init -q ../aiida-quantumespresso && git -C ../aiida-quantumespresso fetch --depth 1 https://github.com/aiidateam/aiida-quantumespresso 0a15b8ac63d35f1422ba0b2184f420c177bd2d70 && git -C ../aiida-quantumespresso checkout -q FETCH_HEAD + git clone --depth 1 --branch patched https://github.com/elinscott/aiida-quantumespresso ../aiida-quantumespresso git clone --depth 1 https://github.com/elinscott/aiida-wannierjl ../aiida-wannierjl git clone --depth 1 https://github.com/elinscott/pydantic-espresso ../pydantic_espresso git init -q ../aiida-core && git -C ../aiida-core fetch --depth 1 https://github.com/aiidateam/aiida-core 4c81e9d64ac561325b91c7e6a18ea2cf792e0609 && git -C ../aiida-core checkout -q FETCH_HEAD diff --git a/.readthedocs.yml b/.readthedocs.yml index cacbd2e..73b240c 100644 --- a/.readthedocs.yml +++ b/.readthedocs.yml @@ -35,7 +35,7 @@ build: - rm -rf ../plumpy && git clone --depth 1 --branch patched https://github.com/elinscott/plumpy ../plumpy - rm -rf ../aiida-wannier90-workflows && git clone --depth 1 --branch patched https://github.com/elinscott/aiida-wannier90-workflows ../aiida-wannier90-workflows - rm -rf ../aiida-wannierjl && git clone --depth 1 https://github.com/elinscott/aiida-wannierjl ../aiida-wannierjl - - git init -q ../aiida-quantumespresso && git -C ../aiida-quantumespresso fetch --depth 1 https://github.com/aiidateam/aiida-quantumespresso 0a15b8ac63d35f1422ba0b2184f420c177bd2d70 && git -C ../aiida-quantumespresso checkout -q FETCH_HEAD + - rm -rf ../aiida-quantumespresso && git clone --depth 1 --branch patched https://github.com/elinscott/aiida-quantumespresso ../aiida-quantumespresso - rm -rf ../pydantic_espresso && git clone --depth 1 https://github.com/elinscott/pydantic-espresso ../pydantic_espresso - git init -q ../aiida-core && git -C ../aiida-core fetch --depth 1 https://github.com/aiidateam/aiida-core 4c81e9d64ac561325b91c7e6a18ea2cf792e0609 && git -C ../aiida-core checkout -q FETCH_HEAD - uv venv $READTHEDOCS_VIRTUALENV_PATH diff --git a/CLAUDE.md b/CLAUDE.md index f00a1bb..7651be0 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -163,7 +163,7 @@ pairing (grouping criterion ↔ screening method) as an equivalence. - Orbital grouping: `group_orbitals_by`/`group_orbitals_tol`. DSCF groups by self-Hartree (kcp.x metric); DFPT groups by wannier90 spread with a per-representative `SCREEN.i_orb` fan-out + alpha broadcast. Criteria and methods are independent in principle; unwired combinations raise. kcw.x's internal `check_spread` shortcut is separate (ak2 graph input, on by default). - Explicit k-paths resolve against the cell's own Bravais lattice (ASE vocabulary, position-insensitive); seekpath only serves automatic paths. - aiida-core: builds from `../aiida-core` at current upstream main (post-v2.8, includes the workgraph-dump MRO fix); aiida-shell rides its git master via `[tool.uv.sources]` until a post-`Code.Model` release exists. CI/RTD clone pin: `4c81e9d6`. NOTE: `uv run --project` re-syncs the venv whenever `../aiida-core` changes — keep that checkout where the CI pin points. -- aiida-quantumespresso: upstream main absorbed every `patched`-branch fix (pdos settings threading, PBC handling, hyperqueue default resources), so the `patched` branch is retired; CI/RTD clone upstream at the pinned sha `0a15b8ac`, and the local checkout tracks upstream main. +- aiida-quantumespresso: the `patched` branch is live again, carrying the two protocol commits that let `get_builder_from_protocol` take a pseudo family recommending no cutoffs (request cutoffs only when the overrides supply none; accept any `aiida-pseudo` family). CI and RTD clone `elinscott/aiida-quantumespresso` at `patched`; the local checkout at `../aiida-quantumespresso` tracks upstream main, so run against a `patched` worktree shadowed on `PYTHONPATH`. - ML descriptors: `self_hartree` wired; `orbital_density` fully built on pw2wannier90 `wan_mode='decompose'` (new ak2 CalcJob+parser, legacy-comparable cross-power, decompose math reproduced to machine precision on live Si) but **gated** pending a live per-block WF-to-alpha alignment regression; flipping the guard is one line. - Known gaps (raise `NotImplementedError` with pointers in `aiida/workflows/`): corrections `PKIPZ`/`NONE`/`ALL`; `init_orbitals='pz'`; `fix_spin_contamination`; gamma-only/molecular DFPT; `eps_inf='auto'` for DSCF (wired for DFPT); `ml: {mode: predict}` with the `power_spectrum` descriptor; UI inside singlepoints (the open item; the `ui-singlepoint` branch is kept as reference); `convergence` task. - Standalone UI (unfold-and-interpolate) is unsupported by decision: `task: unfold_and_interpolate` is rejected at parse (no `Task` enum member), while the `unfold_and_interpolate:` input block and `UnfoldAndInterpolateConfig` stay for the future singlepoint use. diff --git a/src/koopmans/aiida/conversion.py b/src/koopmans/aiida/conversion.py index 1697e11..e8d23c6 100644 --- a/src/koopmans/aiida/conversion.py +++ b/src/koopmans/aiida/conversion.py @@ -6,6 +6,8 @@ from __future__ import annotations +import math +import warnings from pathlib import Path from typing import TYPE_CHECKING, Any @@ -39,6 +41,10 @@ def _convert_paths_to_strings(obj: Any) -> Any: # Quantum ESPRESSO's own value, so that converted quantities match QE output BOHR_TO_ANGSTROM: float = CONSTANTS.bohr_to_ang +# ``ecutrho / ecutwfc`` for a norm-conserving pseudopotential, which is what +# kcp.x and kcw.x accept — and so the only kind koopmans runs. +NORM_CONSERVING_DUAL: float = 4.0 + def code_parallelization( config: CodeParallelization | None, @@ -575,6 +581,54 @@ def kpoints_input_to_kpoints_path( return kpts +def _resolve_pw_cutoffs(system: dict[str, Any], kcp_ecutrho: float) -> None: + """Complete the ``SYSTEM`` cutoff pair in place, from ``ecutwfc``. + + An unstated ``ecutrho`` becomes ``kcp_ecutrho``, or + ``NORM_CONSERVING_DUAL * ecutwfc`` when that is unset too. Whatever its + source, an ``ecutrho`` that is not :data:`NORM_CONSERVING_DUAL` times + ``ecutwfc`` takes effect with a warning naming the key it came from. With + neither cutoff stated the pair is left empty, for the pseudopotential + family to recommend. + + Raises: + ValueError: If ``ecutrho`` is stated and ``ecutwfc`` is not. + """ + ecutwfc = system.get("ecutwfc") + ecutrho = system.get("ecutrho") + source = "calculator_parameters.pw.system.ecutrho" + + if ecutwfc is None: + if ecutrho is not None: + raise ValueError( + "`calculator_parameters.pw.system.ecutrho` is set without a wavefunction " + "cutoff, which would pair it with whatever the pseudopotential family " + "recommends. Set `calculator_parameters.ecutwfc`; `ecutrho` follows at " + f"{NORM_CONSERVING_DUAL:g} times it on its own." + ) + return + + if ecutrho is None: + if kcp_ecutrho: + # An explicit ``kcp.system.ecutrho`` keeps the pw.x runs on the grid + # of the CP supercell run the dft_init consistency checks compare + # against. + ecutrho = kcp_ecutrho + source = "calculator_parameters.kcp.system.ecutrho" + else: + ecutrho = NORM_CONSERVING_DUAL * ecutwfc + system["ecutrho"] = ecutrho + + if not math.isclose(ecutrho, NORM_CONSERVING_DUAL * ecutwfc): + warnings.warn( + f"`{source}` = {ecutrho:g} Ry: ecutrho should be {NORM_CONSERVING_DUAL:g} x " + f"ecutwfc = {NORM_CONSERVING_DUAL * ecutwfc:g} Ry for norm-conserving " + f"pseudopotentials. Drop `{source}` to take that.", + UserWarning, + stacklevel=3, + ) + + def input_to_pw_parameters(koopmans_input: KoopmansInput) -> dict[str, dict[str, Any]]: """Convert KoopmansInput to a PW input-parameter namelist dict. @@ -596,16 +650,6 @@ def input_to_pw_parameters(koopmans_input: KoopmansInput) -> dict[str, dict[str, # Add ecutwfc if specified if calc_params.ecutwfc is not None: parameters["SYSTEM"]["ecutwfc"] = calc_params.ecutwfc - # Pin ecutrho alongside it (kcp.x convention: 4x for norm-conserving, - # same fallback as ``_extract_kcp_scalar_inputs``). Without this the - # pseudo family's placeholder ecutrho wins in protocol-built PW runs, - # putting them on a different grid from the CP supercell run that the - # dft_init consistency checks compare against. An explicit - # ``pw.system.ecutrho`` still overrides via the update below. - kcp_system = calc_params.kcp.system - parameters["SYSTEM"]["ecutrho"] = ( - kcp_system.ecutrho if kcp_system.ecutrho else 4.0 * calc_params.ecutwfc - ) # Add nbnd if specified if calc_params.nbnd is not None: @@ -624,6 +668,11 @@ def input_to_pw_parameters(koopmans_input: KoopmansInput) -> dict[str, dict[str, parameters["ELECTRONS"].update( pw_params.electrons.model_dump(exclude_none=True, exclude_defaults=True) ) + + # After the merge, so the pair is resolved from the cutoffs pw.x will run + # rather than from the top-level shorthand a ``pw.system`` block may replace. + _resolve_pw_cutoffs(parameters["SYSTEM"], calc_params.kcp.system.ecutrho) + # Ensure all Path objects are converted to strings for JSON serialization parameters = _convert_paths_to_strings(parameters) diff --git a/src/koopmans/aiida/setup/pseudos.py b/src/koopmans/aiida/setup/pseudos.py index 2b57f90..d0b6388 100644 --- a/src/koopmans/aiida/setup/pseudos.py +++ b/src/koopmans/aiida/setup/pseudos.py @@ -41,6 +41,22 @@ def ensure_pseudo_family_installed(pseudo_family: str) -> None: logger.info("Successfully installed pseudo family '%s'", pseudo_family) +def pseudo_family_has_cutoffs(pseudo_family: str) -> bool: + """Report whether an installed family publishes recommended cutoffs. + + True only if the family defines at least one cutoff stringency; without one + ``get_recommended_cutoffs`` has nothing to return. + + Raises: + NotExistent: If the family is not installed. + """ + from aiida_pseudo.groups.family import PseudoPotentialFamily + + family = PseudoPotentialFamily.collection.get(label=pseudo_family) + stringencies = getattr(family, "get_cutoff_stringencies", None) + return stringencies is not None and bool(stringencies()) + + def install_pseudo_family(pseudo_family: str) -> None: """Install a pseudopotential family. Parse the label and dispatch.""" parts = pseudo_family.split("/") @@ -179,9 +195,10 @@ def _install_sssp_family(label: str, parts: list[str]) -> None: # SG15 ONCV is published as a single frozen tarball on quantum-simulation.org. It # bundles every version x relativistic variant in one flat archive; the label's # version/relativistic parts select which subset of UPFs to install. There is no -# upstream ``aiida-pseudo`` installer for SG15, so we install as a plain -# ``CutoffsPseudoPotentialFamily`` so recommended cutoffs can be attached later -# via ``family.set_cutoffs`` without a reinstall. +# upstream ``aiida-pseudo`` installer for SG15, so we install a +# ``CutoffsPseudoPotentialFamily`` directly. SG15 publishes no recommended +# cutoffs, so the family carries none and the input file must set ``ecutwfc`` +# and ``ecutrho`` itself. _SG15_ARCHIVE_URL = ( "http://www.quantum-simulation.org/potentials/sg15_oncv/sg15_oncv_upf_2020-02-06.tar.gz" ) diff --git a/src/koopmans/aiida/workflows/__init__.py b/src/koopmans/aiida/workflows/__init__.py index e0ccfdb..c34441e 100644 --- a/src/koopmans/aiida/workflows/__init__.py +++ b/src/koopmans/aiida/workflows/__init__.py @@ -115,6 +115,32 @@ def load_codes_for_task(workflow: WorkflowConfig) -> Codes: return codes +def require_cutoffs_for_family(pseudo_family: str, parameters: dict[str, Any]) -> None: + """Reject an input that names no cutoffs against a family recommending none. + + Args: + pseudo_family: Label of the family the pw.x steps will use. + parameters: The pw.x parameters the input file produced, whose + ``SYSTEM`` block carries ``ecutwfc`` when the input states it. + + Raises: + ValueError: If the family publishes no recommended cutoffs and the + input states none either. + """ + 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." + ) + + def prepare_common_inputs( koopmans_input: KoopmansInput, override_keys: list[str], @@ -123,7 +149,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. An input naming no cutoffs against a + family recommending none is rejected here + (:func:`require_cutoffs_for_family`). Args: koopmans_input: The parsed koopmans input. @@ -140,7 +168,10 @@ def prepare_common_inputs( ensure_pseudo_family_installed(pseudo_family) + require_cutoffs_for_family(pseudo_family, parameters) + pw_overrides: dict[str, Any] = {"parameters": 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). diff --git a/src/koopmans/aiida/workflows/dscf.py b/src/koopmans/aiida/workflows/dscf.py index e5d6a34..cecfb58 100644 --- a/src/koopmans/aiida/workflows/dscf.py +++ b/src/koopmans/aiida/workflows/dscf.py @@ -9,7 +9,7 @@ from aiida_quantumespresso.common.types import SpinType from koopmans.aiida.conversion import atoms_input_to_structure, input_to_pw_parameters -from koopmans.aiida.workflows import load_code, reject_kpoint_overrides +from koopmans.aiida.workflows import load_code, reject_kpoint_overrides, require_cutoffs_for_family from koopmans.aiida.workflows.blocks import ( create_explicit_blocks, validate_blocks_cover_all_occ_bands, @@ -210,6 +210,9 @@ def dscf_wannier_init_inputs( validate_blocks_separate_occ_and_emp(blocks, nocc) validate_blocks_cover_all_occ_bands(blocks, nocc) + # The DSCF route never calls ``prepare_common_inputs``, so the cutoff check + # reaches its pw steps only from here. + require_cutoffs_for_family(pseudo_family, parameters) wannier_overrides: WannierizeOverrides = { "scf": {"pseudo_family": pseudo_family, "pw": {"parameters": parameters}}, "nscf": {"pseudo_family": pseudo_family, "pw": {"parameters": parameters}}, diff --git a/src/koopmans/aiida/workflows/wannierize.py b/src/koopmans/aiida/workflows/wannierize.py index 1d78c82..34d9016 100644 --- a/src/koopmans/aiida/workflows/wannierize.py +++ b/src/koopmans/aiida/workflows/wannierize.py @@ -9,7 +9,11 @@ from aiida_quantumespresso.common.types import SpinType from koopmans.aiida.conversion import atoms_input_to_structure, input_to_pw_parameters -from koopmans.aiida.workflows import pin_step_kpoints, prepare_common_inputs +from koopmans.aiida.workflows import ( + pin_step_kpoints, + prepare_common_inputs, + require_cutoffs_for_family, +) from koopmans.aiida.workflows.blocks import ( create_automatic_blocks, create_explicit_blocks, @@ -310,9 +314,18 @@ def _build_wannierize_blocks_workgraph( scf_parameters.get("SYSTEM", {}).pop("nbnd", None) nscf_parameters = copy.deepcopy(parameters) nscf_parameters.setdefault("SYSTEM", {})["nbnd"] = nbnd + # This route assembles its own scf/nscf overrides instead of calling + # ``prepare_common_inputs``, so the cutoff check is its own too. + require_cutoffs_for_family(pseudo_family, parameters) wannier_overrides: WannierizeOverrides = { - "scf": {"pseudo_family": pseudo_family, "pw": {"parameters": scf_parameters}}, - "nscf": {"pseudo_family": pseudo_family, "pw": {"parameters": nscf_parameters}}, + "scf": { + "pseudo_family": pseudo_family, + "pw": {"parameters": scf_parameters}, + }, + "nscf": { + "pseudo_family": pseudo_family, + "pw": {"parameters": nscf_parameters}, + }, } # User wannier90 keywords (disentanglement windows, iteration counts, ...) diff --git a/src/koopmans/input_file/__init__.py b/src/koopmans/input_file/__init__.py index a530f8d..d201b29 100644 --- a/src/koopmans/input_file/__init__.py +++ b/src/koopmans/input_file/__init__.py @@ -321,7 +321,7 @@ def check_up_down_exclusivity(self) -> Wannier90InputParametersWithUpDown: class CalculatorParametersInput(BaseModel): """Calculator-specific input parameters.""" - ecutwfc: float | None = None + ecutwfc: float | None = Field(default=None, gt=0.0) nbnd: int | None = None tot_magnetization: float | None = None pw: PWInputParameters = Field(default_factory=lambda: PWInputParameters()) @@ -336,6 +336,38 @@ class CalculatorParametersInput(BaseModel): ) kcp: KCPInputParameters = Field(default_factory=lambda: KCPInputParameters()) + @model_validator(mode="after") + def check_cutoffs_agree(self) -> CalculatorParametersInput: + """Validate that the stated wavefunction cutoffs agree, and the density ones. + + pw.x and kcp.x read their cutoffs from different keys, so two stated + values that differ put the two codes on different grids. A + ``kcp.system`` cutoff of zero is unset rather than a stated zero. + + Raises: + ValueError: If two keys state the same cutoff at different values. + """ + wavefunction = { + "calculator_parameters.ecutwfc": self.ecutwfc, + "calculator_parameters.pw.system.ecutwfc": self.pw.system.ecutwfc, + "calculator_parameters.kcp.system.ecutwfc": self.kcp.system.ecutwfc or None, + } + density = { + "calculator_parameters.pw.system.ecutrho": self.pw.system.ecutrho, + "calculator_parameters.kcp.system.ecutrho": self.kcp.system.ecutrho or None, + } + + for cutoff, keys in (("wavefunction", wavefunction), ("density", density)): + stated = {key: value for key, value in keys.items() if value is not None} + if len(set(stated.values())) > 1: + listed = [f"`{key}` = {value:g} Ry" for key, value in stated.items()] + raise ValueError( + f"{', '.join(listed[:-1])} and {listed[-1]} disagree. pw.x and kcp.x " + f"must run on the same grid, so state one {cutoff} cutoff: give these " + "keys the same value, or drop all but one of them." + ) + return self + class KoopmansInput(BaseModel): """Input schema for ``koopmans`` input files.""" @@ -440,6 +472,7 @@ def _resolve(path: str) -> str: "type": 'is the wrong type (should be "{expected_type}", not "{given_type}")', "extra_forbidden": "is not a valid keyword.", "missing": "was not provided.", + "greater_than": "must be greater than {gt}.", } diff --git a/src/koopmans/input_file/pw.py b/src/koopmans/input_file/pw.py index f0fa6ca..9ddd825 100644 --- a/src/koopmans/input_file/pw.py +++ b/src/koopmans/input_file/pw.py @@ -36,7 +36,7 @@ class SystemNamelist(_SystemNamelist): ``ibrav``, ``nat`` and ``ntyp`` are derived from the input structure, not provided by the user. ``ecutwfc`` is optional at parse time; the dispatcher raises if it is - still unset when the workgraph is built. + still unset when the workgraph is built. Both cutoffs must be greater than zero. """ # Excluded fields (see ``ControlNamelist`` above for the ClassVar rationale). @@ -44,7 +44,8 @@ class SystemNamelist(_SystemNamelist): nat: ClassVar[int | None] = None # type: ignore[misc, assignment, unused-ignore] ntyp: ClassVar[int | None] = None # type: ignore[misc, assignment, unused-ignore] # Optional at parse time; the dispatcher raises if still unset at build time. - ecutwfc: float | None = None # type: ignore[assignment] + ecutwfc: float | None = Field(default=None, gt=0.0) # type: ignore[assignment] + ecutrho: float | None = Field(default=None, gt=0.0) class PWInputParameters(BaseModel): diff --git a/tests/conftest.py b/tests/conftest.py index b29ebdb..25edd89 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -20,8 +20,10 @@ compiled_binaries, fake_pseudodojo_lda_family, fake_sg15_cutoffs_family, + fake_sg15_family_without_cutoffs, fake_sg15_fr_cutoffs_family, fake_sg15_pseudo_family, + fake_user_built_family, installed_decompose_code, installed_fold_codes, installed_kcp_code, diff --git a/tests/fixtures.py b/tests/fixtures.py index 768882e..fef014c 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -514,6 +514,7 @@ def _install_fake_family( elements: dict[str, float], cutoffs: bool = False, has_so: bool = False, + recommended_cutoffs: bool = True, ) -> Any: """Install (or fetch) a fake pseudopotential family with synthetic UPF streams. @@ -523,6 +524,8 @@ def _install_fake_family( builders that call ``get_builder_from_protocol`` eagerly at build time (the aiida-qe protocol machinery only accepts SSSP, PseudoDojo, or a cutoffs family); plain families cover ``ensure_pseudo_family_installed``. + ``recommended_cutoffs=False`` leaves that cutoffs family with no stringency + defined, the shape ``_install_sg15_family`` produces. ``has_so=True`` marks every pseudo fully relativistic. """ from aiida.common.exceptions import NotExistent @@ -546,7 +549,7 @@ def _install_fake_family( upf = UpfData(io.BytesIO(content.encode("utf-8")), filename=f"{element}.upf") pseudos.append(upf.store()) family.add_nodes(pseudos) - if cutoffs: + if cutoffs and recommended_cutoffs: family.set_cutoffs( {element: {"cutoff_wfc": 30.0, "cutoff_rho": 240.0} for element in elements}, stringency="normal", @@ -570,6 +573,28 @@ def fake_sg15_cutoffs_family(aiida_profile: Any) -> Any: return _install_fake_family("SG15/1.0/PBE/SR", {"O": 6.0, "Si": 4.0}, cutoffs=True) +@pytest.fixture +def fake_sg15_family_without_cutoffs(aiida_profile: Any) -> Any: + """Install ``SG15/1.2/PBE/FR`` in the shape ``_install_sg15_family`` produces. + + A ``CutoffsPseudoPotentialFamily`` with no stringency defined, so it can + recommend no cutoffs. A label of its own so it coexists with the other + SG15 fixtures in one session profile. + """ + return _install_fake_family( + "SG15/1.2/PBE/FR", {"Si": 4.0}, cutoffs=True, recommended_cutoffs=False + ) + + +@pytest.fixture +def fake_user_built_family(aiida_profile: Any) -> Any: + """Install a plain family, the shape ``aiida-pseudo install family`` produces. + + No cutoff stringencies are even representable on this family class. + """ + return _install_fake_family("MyPseudos/local", {"Si": 4.0}) + + @pytest.fixture def fake_sg15_fr_cutoffs_family(aiida_profile: Any) -> Any: """Install a fake fully-relativistic ``SG15/1.0/PBE/FR`` cutoffs family (O and Si).""" @@ -586,6 +611,52 @@ def fake_pseudodojo_lda_family(aiida_profile: Any) -> Any: return _install_fake_family("PseudoDojo/0.4/LDA/SR/standard/upf", {"Zn": 20.0, "O": 6.0}) +def silicon_pw_input( + *, + pseudo_library: str = "SG15/1.2/PBE/SR", + parallelization: dict[str, Any] | None = None, + calculator_parameters: dict[str, Any] | None = None, + kpoints: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Return a minimal silicon ``dft_bands`` input dict for the wiring tests. + + ``calculator_parameters`` replaces the default ``{"ecutwfc": 20.0}`` block + outright, so a caller can leave the cutoffs out entirely. + """ + d: dict[str, Any] = { + "workflow": {"task": "dft_bands", "pseudo_library": pseudo_library}, + "atoms": { + "cell_parameters": {"periodic": True, "ibrav": 2, "celldms": {"1": 10.2622}}, + "atomic_positions": { + "units": "crystal", + "positions": [["Si", 0.0, 0.0, 0.0], ["Si", 0.25, 0.25, 0.25]], + }, + }, + "kpoints": kpoints or {"grid": [2, 2, 2], "offset": [0, 0, 0]}, + "calculator_parameters": ( + {"ecutwfc": 20.0} if calculator_parameters is None else calculator_parameters + ), + } + if parallelization is not None: + d["parallelization"] = parallelization + return d + + +def pw_step_from_overrides(code: Any, structure: Any, overrides: dict[str, Any]) -> Any: + """Return the ``pw`` sub-builder one scf or nscf step assembles from ``overrides``. + + Mirrors ``aiida_koopmans.workgraphs.pw.assemble_pw_base_step``, which hands + a route's per-step override entry to this builder when the step runs. Use + it on an entry taken off a built graph to see what the pw.x calculation + receives. + """ + from aiida_quantumespresso.workflows.pw.base import PwBaseWorkChain + + return PwBaseWorkChain.get_builder_from_protocol( + code=code, structure=structure, overrides=overrides + ).pw + + def si_external_projector_tables() -> dict[str, list[dict[str, Any]]]: """Return the tables the dispatcher synthesizes from the fixture's ``Si.dat``. diff --git a/tests/test_conversion.py b/tests/test_conversion.py index 4cc11bb..c9fada8 100644 --- a/tests/test_conversion.py +++ b/tests/test_conversion.py @@ -14,6 +14,7 @@ input_to_pw_parameters, ) from koopmans.input_file import AtomsInput +from tests.fixtures import silicon_pw_input as _pw_input SI_ALAT_BOHR = 10.2622 @@ -240,6 +241,46 @@ def test_no_calculation_key(self, aiida_profile: Any) -> None: parameters = input_to_pw_parameters(inp) assert "calculation" not in parameters.get("CONTROL", {}) + def test_ecutrho_follows_the_kcp_block_when_it_is_set(self, aiida_profile: Any) -> None: + """An explicit ``kcp.system.ecutrho`` puts pw.x on the CP supercell grid. + + 100 Ry rather than the four-times-20 the pair would take on its own, + so the assertion cannot be satisfied by the derivation. Off the + norm-conserving ratio it warns as a stated pw pair does, naming the + ``kcp`` key it came from rather than the ``pw`` one the input leaves + unset. + """ + from koopmans.input_file import KoopmansInput + + inp = KoopmansInput.model_validate( + _pw_input( + calculator_parameters={"ecutwfc": 20.0, "kcp": {"system": {"ecutrho": 100.0}}} + ) + ) + with pytest.warns(UserWarning, match=r"`calculator_parameters\.kcp\.system\.ecutrho`"): + parameters = input_to_pw_parameters(inp) + + assert parameters["SYSTEM"]["ecutrho"] == pytest.approx(100.0) + + def test_ecutrho_from_the_kcp_block_on_the_ratio_is_silent(self, aiida_profile: Any) -> None: + """A ``kcp`` density cutoff already at four times ``ecutwfc`` warns about nothing. + + Separates the warning from the source: it is the ratio that is + reported, not the fact that the value came from the ``kcp`` block. + """ + import warnings + + from koopmans.input_file import KoopmansInput + + inp = KoopmansInput.model_validate( + _pw_input(calculator_parameters={"ecutwfc": 20.0, "kcp": {"system": {"ecutrho": 80.0}}}) + ) + with warnings.catch_warnings(): + warnings.simplefilter("error") + parameters = input_to_pw_parameters(inp) + + assert parameters["SYSTEM"]["ecutrho"] == pytest.approx(80.0) + class TestCodeParallelizationHelper: """``code_parallelization`` maps a per-code config to (options, settings).""" @@ -383,30 +424,6 @@ def test_no_config_passes_none(self, aiida_profile: Any, monkeypatch: Any) -> No assert captured["parallelization"] is None -def _pw_input( - *, - pseudo_library: str = "SG15/1.2/PBE/SR", - parallelization: dict[str, Any] | None = None, - kpoints: dict[str, Any] | None = None, -) -> dict[str, Any]: - """Return a minimal silicon dft_bands input dict for the wiring tests.""" - d: dict[str, Any] = { - "workflow": {"task": "dft_bands", "pseudo_library": pseudo_library}, - "atoms": { - "cell_parameters": {"periodic": True, "ibrav": 2, "celldms": {"1": 10.2622}}, - "atomic_positions": { - "units": "crystal", - "positions": [["Si", 0.0, 0.0, 0.0], ["Si", 0.25, 0.25, 0.25]], - }, - }, - "kpoints": kpoints or {"grid": [2, 2, 2], "offset": [0, 0, 0]}, - "calculator_parameters": {"ecutwfc": 20.0}, - } - if parallelization is not None: - d["parallelization"] = parallelization - return d - - class TestDftBandsScfMesh: """The dft_bands scf samples the input file's grid, not a protocol distance.""" diff --git a/tests/test_dscf_mlwf_dispatcher.py b/tests/test_dscf_mlwf_dispatcher.py index 37b84b0..c64ff65 100644 --- a/tests/test_dscf_mlwf_dispatcher.py +++ b/tests/test_dscf_mlwf_dispatcher.py @@ -590,3 +590,73 @@ def test_either_entry_raises_naming_the_grid(self, step: str) -> None: d["kpoints"]["overrides"] = {step: {"grid": [4, 4, 4]}} with pytest.raises(ValueError, match=rf"overrides\.{step}.*`kpoints.grid`"): _build(d, codes={}) + + +class TestCutoffLessPseudoFamily: + """A family recommending no cutoffs drives this route's pw steps from the input. + + The Wannier initialisation builds its own scf and nscf overrides rather + than going through ``prepare_common_inputs``, so it carries the cutoff + check of its own. The checks drive the built graph's override entries + into the pw protocol builder, which is what the steps do when they run. + """ + + @staticmethod + def _si_structure(d: dict[str, Any]) -> Any: + from koopmans.aiida.conversion import atoms_input_to_structure + + return atoms_input_to_structure(KoopmansInput.model_validate(d).atoms) + + @pytest.mark.parametrize("step", ["scf", "nscf"]) + def test_the_step_takes_the_input_cutoffs_and_the_family_pseudos( + self, + aiida_profile_clean: Any, + dscf_codes: Any, + fake_sg15_family_without_cutoffs: Any, + step: str, + ) -> None: + """Both cutoffs and the family's own pseudos reach the pw.x calculation. + + The family recommends nothing, so 20 Ry and its derived 80 Ry can only + have come from the input; the pseudo uuids say the builder resolved + them against the named family rather than some other one. + """ + from tests.fixtures import pw_step_from_overrides + + d = _si_dscf_dict(pseudo_library=fake_sg15_family_without_cutoffs.label) + wg = _build(d, dscf_codes) + structure = self._si_structure(d) + + entry = wg.tasks["wannier_initialization"].inputs["wannier_overrides"][step].value + pw = pw_step_from_overrides(dscf_codes["pw"], structure, entry) + + assert pw.parameters["SYSTEM"]["ecutwfc"] == pytest.approx(20.0) + assert pw.parameters["SYSTEM"]["ecutrho"] == pytest.approx(80.0) + expected = fake_sg15_family_without_cutoffs.get_pseudos(structure=structure) + assert pw.pseudos["Si"].uuid == expected["Si"].uuid + + def test_no_cutoffs_at_all_names_the_family_and_the_keyword( + self, + aiida_profile_clean: Any, + dscf_codes: Any, + fake_sg15_family_without_cutoffs: Any, + ) -> None: + """A cutoff stated for kcp.x alone leaves the pw.x steps with none. + + ``calculator_parameters.kcp.system.ecutwfc`` satisfies the kcp.x + requirement but never reaches the pw.x parameters, so this is the + input that arrives at the family with nothing to state. The route + reaches ``require_cutoffs_for_family`` nowhere else: drop the call and + the graph builds without complaint, carrying no wavefunction cutoff at + all into its pw.x steps. + """ + d = _si_dscf_dict(pseudo_library=fake_sg15_family_without_cutoffs.label) + del d["calculator_parameters"]["ecutwfc"] + d["calculator_parameters"]["kcp"] = {"system": {"ecutwfc": 20.0}} + + with pytest.raises(ValueError) as excinfo: + _build(d, dscf_codes) + + message = str(excinfo.value) + assert fake_sg15_family_without_cutoffs.label in message + assert "calculator_parameters.ecutwfc" in message diff --git a/tests/test_error_translation.py b/tests/test_error_translation.py index f39d6a6..91317b6 100644 --- a/tests/test_error_translation.py +++ b/tests/test_error_translation.py @@ -16,7 +16,7 @@ from koopmans.aiida.workflows import advice_for, build_workgraph from koopmans.input_file import KoopmansInput -from tests.test_conversion import _pw_input +from tests.fixtures import silicon_pw_input as _pw_input from tests.test_dfpt_dispatcher import _si_dfpt_dict from tests.test_dscf_mlwf_dispatcher import _si_dscf_dict from tests.test_trajectory_dispatcher import _trajectory_input_dict diff --git a/tests/test_input_file.py b/tests/test_input_file.py index 4ee757b..5d85aa9 100644 --- a/tests/test_input_file.py +++ b/tests/test_input_file.py @@ -133,12 +133,12 @@ def _minimal_si_input() -> dict[str, object]: ] -def _set_removed_keyword(d: dict[str, object], section: str, keyword: str) -> None: - """Set ``keyword`` under the (dotted) ``section`` path of an input dict.""" +def _set_keyword(d: dict[str, object], section: str, keyword: str, value: object = True) -> None: + """Set ``keyword`` to ``value`` under the (dotted) ``section`` path of an input dict.""" target = d for part in section.split("."): target = target.setdefault(part, {}) # type: ignore[assignment] - target[keyword] = True + target[keyword] = value class TestRemovedKeywordsRejected: @@ -150,7 +150,7 @@ def test_removed_keyword_rejected_by_model(self, section: str, keyword: str) -> from pydantic import ValidationError d = _minimal_si_input() - _set_removed_keyword(d, section, keyword) + _set_keyword(d, section, keyword) with pytest.raises(ValidationError, match="Extra inputs are not permitted"): KoopmansInput.model_validate(d) @@ -161,7 +161,7 @@ def test_removed_keyword_gives_friendly_message( ) -> None: """``read_input_file`` reports the retired keyword as an invalid keyword.""" d = _minimal_si_input() - _set_removed_keyword(d, section, keyword) + _set_keyword(d, section, keyword) input_file = tmp_path / "input.json" input_file.write_text(json.dumps(d)) @@ -169,6 +169,149 @@ def test_removed_keyword_gives_friendly_message( read_input_file(input_file) +_CUTOFF_KEYS = [ + ("calculator_parameters.pw.system", "ecutwfc"), + ("calculator_parameters.pw.system", "ecutrho"), + ("calculator_parameters", "ecutwfc"), +] + + +class TestCutoffsMustBePositive: + """A cutoff of zero or less is rejected at parse.""" + + @pytest.mark.parametrize(("section", "keyword"), _CUTOFF_KEYS) + @pytest.mark.parametrize("value", [0.0, -45.0]) + def test_a_non_positive_cutoff_is_rejected( + self, section: str, keyword: str, value: float, tmp_path: Path + ) -> None: + """The message names the key the input file used, not a ratio arithmetic failure. + + Zero and a negative value fail differently downstream — one divides by + zero building the off-ratio warning, the other reports a ratio of -36 — + so both are asserted rather than one standing in for the pair. + """ + d = _minimal_si_input() + _set_keyword(d, section, keyword, value) + input_file = tmp_path / "input.json" + input_file.write_text(json.dumps(d)) + + with pytest.raises(ValueError) as excinfo: + read_input_file(input_file) + + message = str(excinfo.value) + assert f"`{section}.{keyword}`" in message + assert "must be greater than 0" in message + + +def _si_input_with(calculator_parameters: dict[str, object]) -> dict[str, object]: + """Return the minimal silicon input, its ``calculator_parameters`` replaced.""" + d = _minimal_si_input() + d["calculator_parameters"] = calculator_parameters + return d + + +_DISAGREEING_CUTOFFS = [ + ( + "pw and kcp state different wavefunction cutoffs", + {"pw": {"system": {"ecutwfc": 45.0}}, "kcp": {"system": {"ecutwfc": 20.0}}}, + ["calculator_parameters.pw.system.ecutwfc", "calculator_parameters.kcp.system.ecutwfc"], + ), + ( + "pw and kcp state different density cutoffs", + { + "ecutwfc": 45.0, + "pw": {"system": {"ecutrho": 180.0}}, + "kcp": {"system": {"ecutrho": 300.0}}, + }, + ["calculator_parameters.pw.system.ecutrho", "calculator_parameters.kcp.system.ecutrho"], + ), + ( + "a pw block overrides the shorthand kcp falls back to", + {"ecutwfc": 20.0, "nbnd": 8, "pw": {"system": {"ecutwfc": 45.0}}}, + ["calculator_parameters.ecutwfc", "calculator_parameters.pw.system.ecutwfc"], + ), + ( + "a kcp block states what the shorthand overrides", + {"ecutwfc": 20.0, "kcp": {"system": {"ecutwfc": 30.0}}}, + ["calculator_parameters.ecutwfc", "calculator_parameters.kcp.system.ecutwfc"], + ), +] + +_AGREEING_CUTOFFS = [ + ("the shorthand alone", {"ecutwfc": 20.0}), + ("a pw block restating the shorthand", {"ecutwfc": 20.0, "pw": {"system": {"ecutwfc": 20.0}}}), + ( + "pw and kcp stating one wavefunction cutoff", + {"pw": {"system": {"ecutwfc": 45.0}}, "kcp": {"system": {"ecutwfc": 45.0}}}, + ), + ( + "pw and kcp stating one density cutoff", + { + "ecutwfc": 45.0, + "pw": {"system": {"ecutrho": 180.0}}, + "kcp": {"system": {"ecutrho": 180.0}}, + }, + ), + ("a pw cutoff with no kcp block", {"pw": {"system": {"ecutwfc": 45.0}}}), + ( + "a kcp density cutoff with no pw block", + {"ecutwfc": 40.0, "kcp": {"system": {"ecutrho": 160.0}}}, + ), +] + + +class TestCutoffsMustAgreeAcrossBlocks: + """pw.x and kcp.x read their cutoffs from different keys, and must get one grid.""" + + @pytest.mark.parametrize( + ("label", "calculator_parameters", "keys"), + _DISAGREEING_CUTOFFS, + ids=[case[0] for case in _DISAGREEING_CUTOFFS], + ) + def test_disagreeing_cutoffs_are_rejected( + self, + label: str, + calculator_parameters: dict[str, object], + keys: list[str], + tmp_path: Path, + ) -> None: + """The message names every key that stated a value, so the user can pick one. + + The shorthand cases are the reproduced defect: an input naming only + ``pw.system.ecutwfc`` beside the shorthand ran pw.x at 45 Ry and kcp.x + at 20 Ry, and neither block stated the cutoff twice. + """ + input_file = tmp_path / "input.json" + input_file.write_text(json.dumps(_si_input_with(calculator_parameters))) + + with pytest.raises(ValueError) as excinfo: + read_input_file(input_file) + + message = str(excinfo.value) + for key in keys: + assert f"`{key}`" in message + assert "must run on the same grid" in message + + @pytest.mark.parametrize( + ("label", "calculator_parameters"), + _AGREEING_CUTOFFS, + ids=[case[0] for case in _AGREEING_CUTOFFS], + ) + def test_agreeing_cutoffs_are_accepted( + self, label: str, calculator_parameters: dict[str, object], tmp_path: Path + ) -> None: + """Restating a cutoff, and leaving one block silent, both stay legal. + + Separates disagreement from mere repetition: a rule keyed on "two + blocks mention it" rather than on the values would reject these, and + the last case is the shape the O2 tutorial ships. + """ + input_file = tmp_path / "input.json" + input_file.write_text(json.dumps(_si_input_with(calculator_parameters))) + + read_input_file(input_file) + + def _parallelization_input(*, parallelization: object | None = None) -> dict[str, object]: """Return a minimal silicon input dict for parallelization-block tests.""" d: dict[str, object] = { diff --git a/tests/test_pseudo_cutoffs.py b/tests/test_pseudo_cutoffs.py new file mode 100644 index 0000000..33710d3 --- /dev/null +++ b/tests/test_pseudo_cutoffs.py @@ -0,0 +1,250 @@ +"""A pseudo family publishing no recommended cutoffs takes them from the input.""" + +from __future__ import annotations + +from typing import Any + +import pytest + +from koopmans.input_file import KoopmansInput +from tests.fixtures import silicon_pw_input + + +def _build_scf_pw(code: Any, structure: Any, overrides: dict[str, Any]) -> Any: + """Return the scf pw sub-builder ``PwBandsWorkChain`` assembles.""" + from aiida_quantumespresso.workflows.pw.bands import PwBandsWorkChain + + builder = PwBandsWorkChain.get_builder_from_protocol( + code=code, structure=structure, overrides=overrides + ) + return builder.scf.pw + + +class TestFamilyWithCutoffs: + """A family that recommends cutoffs keeps recommending them.""" + + def test_an_input_may_still_omit_the_cutoffs( + self, aiida_profile_clean: Any, installed_pw_code: Any, fake_sg15_cutoffs_family: Any + ) -> None: + """An input naming no cutoffs at all builds on the family's recommendation. + + Discriminates the added branch from a replacement of the old one: if + the pseudos were pinned unconditionally, this input would be rejected + for the cutoffs it does not state. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input(pseudo_library="SG15/1.0/PBE/SR", calculator_parameters={}) + ) + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + assert "pseudos" not in overrides["scf"]["pw"] + + recommended = fake_sg15_cutoffs_family.get_recommended_cutoffs( + structure=structure, unit="Ry" + ) + system = _build_scf_pw(installed_pw_code, structure, overrides).parameters["SYSTEM"] + assert (system["ecutwfc"], system["ecutrho"]) == pytest.approx(recommended) + + def test_the_input_cutoffs_beat_the_recommendation( + self, aiida_profile_clean: Any, installed_pw_code: Any, fake_sg15_cutoffs_family: Any + ) -> None: + """Cutoffs stated in the input displace the family's, which are lower here. + + The fixture family recommends 30 eV / 240 eV, i.e. about 2.2 Ry / + 17.6 Ry, so an input at 45 Ry / 180 Ry is nowhere near it: the + assertion separates the two sources rather than reading a value both + could have produced. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input( + pseudo_library="SG15/1.0/PBE/SR", + calculator_parameters={"pw": {"system": {"ecutwfc": 45.0, "ecutrho": 180.0}}}, + ) + ) + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + + system = _build_scf_pw(installed_pw_code, structure, overrides).parameters["SYSTEM"] + assert (system["ecutwfc"], system["ecutrho"]) == pytest.approx((45.0, 180.0)) + + def test_ecutwfc_alone_carries_its_own_ecutrho( + self, aiida_profile_clean: Any, installed_pw_code: Any, fake_sg15_cutoffs_family: Any + ) -> None: + """``ecutwfc`` alone runs against four times itself, not the recommendation. + + The reproduced defect: the recommended 17.6 Ry beside an input's 45 Ry + is a ratio of 0.4, a pair neither source asked for. 180 Ry is far from + 17.6 Ry, so reading the derived value distinguishes the two sources — + as merely surviving the build would not. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input( + pseudo_library="SG15/1.0/PBE/SR", + calculator_parameters={"pw": {"system": {"ecutwfc": 45.0}}}, + ) + ) + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + + system = _build_scf_pw(installed_pw_code, structure, overrides).parameters["SYSTEM"] + assert (system["ecutwfc"], system["ecutrho"]) == pytest.approx((45.0, 180.0)) + + def test_the_pair_is_resolved_after_the_pw_block_is_merged( + self, aiida_profile_clean: Any, installed_pw_code: Any, fake_sg15_cutoffs_family: Any + ) -> None: + """A shorthand ``ecutwfc`` and a ``pw`` block ``ecutrho`` are one pair. + + The two cutoffs arrive from different keys, so resolving before the + merge would derive 180 Ry from the shorthand, then overwrite it with + 300 Ry and never measure the ratio the two make: the warning is what + says the merged pair was the one checked. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input( + pseudo_library="SG15/1.0/PBE/SR", + calculator_parameters={"ecutwfc": 45.0, "pw": {"system": {"ecutrho": 300.0}}}, + ) + ) + with pytest.warns(UserWarning, match="norm-conserving"): + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + + system = _build_scf_pw(installed_pw_code, structure, overrides).parameters["SYSTEM"] + assert (system["ecutwfc"], system["ecutrho"]) == pytest.approx((45.0, 300.0)) + + def test_ecutrho_alone_is_rejected( + self, aiida_profile_clean: Any, fake_sg15_cutoffs_family: Any + ) -> None: + """A density cutoff alone would pair with a recommended wavefunction cutoff. + + Inverting the ratio is arithmetic, not intent: ``ecutwfc`` is the + cutoff a plane-wave calculation is converged against, and an input + that names only the other one has left it out by mistake. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input( + pseudo_library="SG15/1.0/PBE/SR", + calculator_parameters={"pw": {"system": {"ecutrho": 180.0}}}, + ) + ) + with pytest.raises(ValueError) as excinfo: + prepare_common_inputs(inp, ["scf", "bands"]) + + message = str(excinfo.value) + assert "`calculator_parameters.pw.system.ecutrho`" in message + assert "`calculator_parameters.ecutwfc`" in message + + def test_a_stated_pair_off_the_ratio_warns_and_is_kept( + self, aiida_profile_clean: Any, installed_pw_code: Any, fake_sg15_cutoffs_family: Any + ) -> None: + """Both cutoffs stated take effect, at whatever ratio, with a warning. + + Nothing is half-taken here — the input names both — so the pair is not + refused; the warning is what says a ratio of 6.67 is not a + norm-conserving one. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input( + pseudo_library="SG15/1.0/PBE/SR", + calculator_parameters={"pw": {"system": {"ecutwfc": 45.0, "ecutrho": 300.0}}}, + ) + ) + with pytest.warns(UserWarning, match="norm-conserving"): + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + + system = _build_scf_pw(installed_pw_code, structure, overrides).parameters["SYSTEM"] + assert (system["ecutwfc"], system["ecutrho"]) == pytest.approx((45.0, 300.0)) + + +class TestFamilyWithoutCutoffs: + """Both shapes an uncut family takes are driven from the input file.""" + + @pytest.mark.parametrize( + ("fixture", "label"), + [ + ("fake_sg15_family_without_cutoffs", "SG15/1.2/PBE/FR"), + ("fake_user_built_family", "MyPseudos/local"), + ], + ) + def test_input_cutoffs_reach_the_builder( + self, + aiida_profile_clean: Any, + installed_pw_code: Any, + request: Any, + fixture: str, + label: str, + ) -> None: + """With both cutoffs given, the build succeeds and carries them. + + Covers the family koopmans installs itself (a cutoffs family with no + stringency) and the one ``aiida-pseudo install family`` produces (a + plain family, which cannot carry cutoffs at all). ``ecutrho`` at 80 Ry + is the top-level ``ecutwfc`` shorthand at four times the input's 20 Ry, + so it can have come from nowhere else. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + family = request.getfixturevalue(fixture) + inp = KoopmansInput.model_validate(silicon_pw_input(pseudo_library=label)) + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + + pw = _build_scf_pw(installed_pw_code, structure, overrides) + assert pw.parameters["SYSTEM"]["ecutwfc"] == pytest.approx(20.0) + assert pw.parameters["SYSTEM"]["ecutrho"] == pytest.approx(80.0) + assert pw.pseudos["Si"].uuid == family.get_pseudos(structure=structure)["Si"].uuid + + def test_a_pw_block_ecutwfc_alone_is_enough( + self, + aiida_profile_clean: Any, + installed_pw_code: Any, + fake_sg15_family_without_cutoffs: Any, + ) -> None: + """A family with nothing to recommend is satisfied by ``ecutwfc`` alone. + + The builder has no recommendation to fall back on, so the derived + 80 Ry is the only ``ecutrho`` in play. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input( + pseudo_library="SG15/1.2/PBE/FR", + calculator_parameters={"pw": {"system": {"ecutwfc": 20.0}}}, + ) + ) + structure, _, overrides = prepare_common_inputs(inp, ["scf", "bands"]) + + pw = _build_scf_pw(installed_pw_code, structure, overrides) + assert pw.parameters["SYSTEM"]["ecutwfc"] == pytest.approx(20.0) + assert pw.parameters["SYSTEM"]["ecutrho"] == pytest.approx(80.0) + assert "Si" in pw.pseudos + + def test_no_cutoffs_at_all_names_the_family_and_the_keyword( + self, aiida_profile_clean: Any, fake_sg15_family_without_cutoffs: Any + ) -> None: + """An input stating neither cutoff fails on koopmans' own message. + + Nothing can supply them: the family recommends none. The message must + not repeat upstream's claim that the family, installed moments + earlier, is not installed. + """ + from koopmans.aiida.workflows import prepare_common_inputs + + inp = KoopmansInput.model_validate( + silicon_pw_input(pseudo_library="SG15/1.2/PBE/FR", calculator_parameters={}) + ) + with pytest.raises(ValueError) as excinfo: + prepare_common_inputs(inp, ["scf", "bands"]) + + message = str(excinfo.value) + assert "SG15/1.2/PBE/FR" in message + assert "calculator_parameters.ecutwfc" in message + assert "is not installed" not in message diff --git a/tests/test_singlepoint_dispatcher.py b/tests/test_singlepoint_dispatcher.py index a70ecc4..66a4197 100644 --- a/tests/test_singlepoint_dispatcher.py +++ b/tests/test_singlepoint_dispatcher.py @@ -149,6 +149,52 @@ def test_workflow_fields_forwarded(self, ozone_input: KoopmansInput) -> None: assert inputs["initial_alpha"] == workflow.alpha_guess +_ACCEPTED_CUTOFF_SHAPES = [ + ("the shorthand alone", {"ecutwfc": 45.0}), + ("a pw block restating the shorthand", {"ecutwfc": 45.0, "pw.system.ecutwfc": 45.0}), + ("pw and kcp stating one cutoff", {"pw.system.ecutwfc": 45.0, "kcp.system.ecutwfc": 45.0}), + ("a kcp density cutoff", {"ecutwfc": 45.0, "kcp.system.ecutrho": 180.0}), + ("a pw density cutoff", {"ecutwfc": 45.0, "pw.system.ecutrho": 180.0}), +] + + +class TestPwAndKcpCutoffsAgree: + """Every input the schema accepts puts pw.x and kcp.x on one grid.""" + + @pytest.mark.parametrize( + ("label", "overrides"), + _ACCEPTED_CUTOFF_SHAPES, + ids=[case[0] for case in _ACCEPTED_CUTOFF_SHAPES], + ) + def test_both_codes_receive_the_same_pair( + self, + ozone_input: KoopmansInput, + aiida_profile: object, + label: str, + overrides: dict[str, float], + ) -> None: + """Reads the numbers each code is handed, rather than trusting the parse check. + + The schema rejects an input that states two different cutoffs, which + says nothing about the ones it accepts: pw.x resolves its pair from the + ``pw`` block and kcp.x from the ``kcp`` block, by separate rules that a + shared input value is no guarantee of. Each shape below reaches the two + resolvers by a different route. + """ + from koopmans.aiida.conversion import input_to_pw_parameters + + # The tutorial's own shorthand is cleared first, so each shape states + # only the keys it names. + inp = _copy_with_calc_overrides(ozone_input, **{"ecutwfc": None, **overrides}) + + pw_system = input_to_pw_parameters(inp)["SYSTEM"] + kcp = kcp_dscf_inputs(inp) + + assert pw_system["ecutwfc"] == pytest.approx(kcp["ecutwfc"]) + assert pw_system["ecutrho"] == pytest.approx(kcp["ecutrho"]) + assert (kcp["ecutwfc"], kcp["ecutrho"]) == pytest.approx((45.0, 180.0)) + + class TestBuildSinglepointWorkgraphScopeGuards: """Scope-guard tests for ``build_singlepoint_workgraph``. diff --git a/tests/test_wannierize_blocks_dispatcher.py b/tests/test_wannierize_blocks_dispatcher.py index 8028b88..4dc31b8 100644 --- a/tests/test_wannierize_blocks_dispatcher.py +++ b/tests/test_wannierize_blocks_dispatcher.py @@ -1047,3 +1047,71 @@ def test_scf_samples_the_input_mesh( assert list(scf_kpoints.get_kpoints_mesh()[0]) == [2, 2, 2] # The nscf keeps the unreduced expansion of the same grid. assert len(wg.tasks["scf_nscf"].inputs["nscf_kpoints"].value.get_kpoints()) == _NUM_KPOINTS + + +class TestCutoffLessPseudoFamily: + """A family recommending no cutoffs drives this route's pw steps from the input. + + The route builds its own scf and nscf overrides rather than going through + ``prepare_common_inputs``, so it carries the cutoff check of its own. The + checks drive the built graph's override entries into the pw protocol + builder, which is what the steps do when they run. + """ + + @staticmethod + def _si_structure(d: dict[str, Any]) -> Any: + from koopmans.aiida.conversion import atoms_input_to_structure + + return atoms_input_to_structure(KoopmansInput.model_validate(d).atoms) + + @pytest.mark.parametrize("step", ["scf", "nscf"]) + def test_the_step_takes_the_input_cutoffs_and_the_family_pseudos( + self, + aiida_profile_clean: Any, + split_codes: Any, + fake_sg15_family_without_cutoffs: Any, + step: str, + ) -> None: + """Both cutoffs and the family's own pseudos reach the pw.x calculation. + + The family recommends nothing, so 20 Ry and its derived 80 Ry can only + have come from the input; the pseudo uuids say the builder resolved + them against the named family rather than some other one. + """ + from tests.fixtures import pw_step_from_overrides + + d = _si_split_dict(pseudo_library=fake_sg15_family_without_cutoffs.label) + wg = _build(d, split_codes) + structure = self._si_structure(d) + + overrides = wg.tasks["scf_nscf"].inputs["overrides"].value + pw = pw_step_from_overrides(split_codes["pw"], structure, overrides[step]) + + assert pw.parameters["SYSTEM"]["ecutwfc"] == pytest.approx(20.0) + assert pw.parameters["SYSTEM"]["ecutrho"] == pytest.approx(80.0) + expected = fake_sg15_family_without_cutoffs.get_pseudos(structure=structure) + assert pw.pseudos["Si"].uuid == expected["Si"].uuid + + def test_no_cutoffs_at_all_names_the_family_and_the_keyword( + self, + aiida_profile_clean: Any, + split_codes: Any, + fake_sg15_family_without_cutoffs: Any, + ) -> None: + """An input stating neither cutoff fails on koopmans' own message. + + The route reaches ``require_cutoffs_for_family`` nowhere else, so + dropping the call leaves the user with aiida-quantumespresso's + account of the same input — stringencies and ``overrides``, neither + of them an input-file keyword. The two assertions name what only the + koopmans message says. + """ + d = _si_split_dict(pseudo_library=fake_sg15_family_without_cutoffs.label) + del d["calculator_parameters"]["ecutwfc"] + + with pytest.raises(ValueError) as excinfo: + _build(d, split_codes) + + message = str(excinfo.value) + assert fake_sg15_family_without_cutoffs.label in message + assert "calculator_parameters.ecutwfc" in message