Skip to content

Add koopmans pseudos to list the valid pseudo libraries - #128

Merged
elinsc-bot merged 12 commits into
mainfrom
k2-pseudos-command
Aug 10, 2026
Merged

Add koopmans pseudos to list the valid pseudo libraries#128
elinsc-bot merged 12 commits into
mainfrom
k2-pseudos-command

Conversation

@elinsc-bot

@elinsc-bot elinsc-bot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Problem

koopmans pseudos listed the pseudopotential libraries koopmans can install, but it built the SG15 labels by crossing {1.0, 1.2} with {SR, FR} rather than from what the archive publishes. Two consequences:

  • SG15/1.2/PBE/FR was offered and cannot be installed. No member of the archive matches it. Asking for it downloads 6 MB and then fails with "No UPF files matched … The archive layout may have changed".
  • SG15/1.1 was never offered, though the archive carries it. It is the only source of fully relativistic Si, P, S, Cl, As, Se, Sb, Te, I, Xe, Ar and Sn — so spin-orbit work on any of those had no SG15 route at all.

The accompanying note also told users that "SG15/1.0 carries only H and O". It carries all 69 elements, the same set as 1.2. That claim came from a hand-built family on a developer machine, not from the archive.

Changes

  • koopmans pseudos lists the libraries koopmans can install, with a note per library.

  • The SG15 versions and variants come from one table of what the archive publishes — 1.0: SR, FR, 1.1: SR, FR, 1.2: SR — which both the listing and the installer read. SG15/1.2/PBE/FR is no longer offered, and asking for it is refused before the download rather than after it, naming what 1.2 does carry.

  • SG15/1.1 becomes installable, composed over 1.0. SG15 published 1.1 as a delta revising 17 elements, so the label installs the 1.0 files with the 1.1 ones over them — a complete set, not the delta. Composed, SG15/1.1/PBE/FR is the maximal fully relativistic SG15 set at 64 elements and puts FR Si and FR O under one label; a run mixing them previously needed two families. Ba, Be, Bi, Li and Ne stay uncovered at every version.

  • The notes say what the archive contains: 1.2 and 1.0 both cover all 69 elements at different revisions, 1.1 is composed and says so, and there is no fully relativistic set at 1.2.

  • Installed pseudopotentials keep their archive filenames (Si_ONCV_PBE-1.1.upf, not Si.upf), which is what makes a composed family self-describing on disk.

  • Cutoffs: the note asks for ecutwfc alone, since ecutrho follows at four times it.

  • The SSSP refusal said SSSP "mixes ultrasoft and PAW". It also carries norm-conserving entries for some elements, so the refusal now names all three.

  • Say what a rejected pseudo_library label means #109 (merged since) listed SSSP among the families koopmans downloads, in the error message, the help text and the docs — two branches above the code that refuses it. All three places now offer only the norm-conserving PseudoDojo and SG15 families, with SSSP kept as the example of a refused library.

  • The unrecognized-label message closes by pointing at koopmans pseudos instead of quoting the label grammars. The listing cannot go stale (PseudoDojo labels come from get_valid_labels()), and the grammar ended in /format while only upf is accepted — following it verbatim produced a label the installer refuses.

  • Self-installed families are now verified norm-conserving from their UPF headers (upf-tools), at use time, on the elements the structure uses. The download routes need no such sweep: PseudoDojo publishes its PAW sets as jthxml only, so the UPF-only filter already guarantees every offered label is ONCV; SSSP stays refused by name, before any download. The check refuses on positive evidence only.

  • The header read is ~40 lines of our own, not a dependency: aiida-pseudo parses only the element and z_valence, and upf-tools' XML reader rejects every PSlibrary pseudopotential whose PP_INFO embeds a Fortran namelist (the bare & breaks XML) — including three of aiida-core's own ultrasoft fixtures, which an accept-on-unreadable rule would then wave through. Validated against 75 real pseudopotentials: 6 non-norm-conserving refused, 69 accepted, none wrong.

  • pseudos.py became a pseudos/ package: one module per library exposing available_labels/install, a registry keyed on the label's first segment, and the header check as shared plumbing. Pure move in its own commit.

  • The hard-coded set_default_stringency("normal") stays, now with a comment: aiida-pseudo promotes only a family's first stringency to default, so dropping the call would leave every koopmans-installed PseudoDojo family on low, and normal matches aiida-pseudo's own CLI default.

Testing

  • The element counts come from enumerating the checksummed 2020-02-06 tarball with the module's own filename pattern, after verifying its SHA-256 against the pinned value. 219 files, all matching; nothing was inferred from an installed family.

  • Reinstating the old notes and the {1.0, 1.2} × {SR, FR} cross product fails five of the new tests, one per claim — the H-and-O note, the fully relativistic coverage, the ecutrho ask, the set of offered labels, and the pre-download guard. That control is what makes the new assertions load-bearing rather than restatements.

  • The discriminating pair for composition: composed 1.1/FR contains Si and O while pure 1.0/FR lacks Si, and the composed SR family carries 1.1's Si content beside 1.0's O — so dropping composition, leaking it into 1.0, or flipping the overlay precedence each fail a different assertion.

  • Run against the patched aiida-quantumespresso the repo clones, confirmed at runtime rather than assumed.

@read-the-docs-community

read-the-docs-community Bot commented Aug 6, 2026

Copy link
Copy Markdown

@elinsc-bot
elinsc-bot deleted the branch main August 7, 2026 16:44
@elinsc-bot elinsc-bot closed this Aug 7, 2026
@elinsc-bot elinsc-bot reopened this Aug 7, 2026
@elinsc-bot
elinsc-bot changed the base branch from k2-pseudo-cutoffs to main August 7, 2026 17:01
elinsc-bot and others added 2 commits August 7, 2026 19:03
The `pseudo_library` field's description sent users to `koopmans pseudos
list`, a command of the previous package that this one does not have, and
an invalid value was answered with the label's grammar rather than the
values that exist.

- Print every family a user can name, grouped by library and marked when
  installed.
- Ask PseudoDojo and SSSP for their own labels rather than writing them
  down; SG15, which is not an `aiida-pseudo` library, is enumerated from
  the installer's supported versions and relativistic variants.
- Note that SG15/1.0 carries only H and O, and that SG15 recommends no
  cutoffs.
- Point the field description and the ozone tutorial at the command.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`koopmans pseudos` listed 61 labels, of which 44 name families no
koopmans calculation can use: SSSP's 14 mix ultrasoft and PAW
pseudopotentials, and 30 of PseudoDojo's 43 are in formats
`PwCalculation` does not accept. The listing is now the 13 PseudoDojo
UPF labels plus SG15's 4.

- Drop SSSP from the listing, the field description and the installer.
- Keep only PseudoDojo's UPF labels, in the listing and the installer.
- Refuse an SSSP or non-UPF label at install with a message naming the
  constraint, rather than installing a family that cannot be used.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.07113% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.85%. Comparing base (815c65d) to head (4e8393c).

Files with missing lines Patch % Lines
...c/koopmans/aiida/setup/pseudos/_norm_conserving.py 92.30% 4 Missing and 1 partial ⚠️
src/koopmans/aiida/setup/pseudos/_sg15.py 97.01% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #128      +/-   ##
==========================================
+ Coverage   81.86%   83.85%   +1.99%     
==========================================
  Files          44       47       +3     
  Lines        3462     3581     +119     
  Branches      542      561      +19     
==========================================
+ Hits         2834     3003     +169     
+ Misses        493      450      -43     
+ Partials      135      128       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

elinsc-bot and others added 5 commits August 7, 2026 19:18
The listing crossed every SG15 version with every relativistic variant,
so it offered `SG15/1.2/PBE/FR`. Installing that label downloaded the
6 MB tarball and matched none of its members, failing on "No UPF files
matched". The archive holds no `_FR-1.2.upf` file at all.

- Read the offered labels from a per-version table of the relativistic
  variants the 2020-02-06 tarball carries, rather than a cross product.
- Add 1.1, which the tarball publishes and the old version set omitted;
  it is the only source of fully relativistic Si, P, S and Cl.
- Refuse a version/variant pair the archive lacks before downloading,
  naming the variants that version does carry.
- Replace the note claiming 1.0 carries only H and O. Enumerating the
  tarball by the installer's own filename pattern gives 69
  scalar-relativistic elements at 1.0 and at 1.2, 17 at 1.1, and fully
  relativistic files at 1.0 (52 elements) and 1.1 (12) alone.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three statements outlived the changes they described.

- The SG15 note told users to set both `ecutwfc` and `ecutrho`; koopmans
  now derives `ecutrho` at four times `ecutwfc` for norm-conserving
  pseudopotentials, so naming both invites an input it would reject.
- Two test comments said the aiida-quantumespresso protocol machinery
  accepts PseudoDojo or a cutoffs family alone. It takes any
  aiida-pseudo family; what it still needs is the cutoffs, from the
  family or from the overrides.
- The SSSP refusal called SSSP a mix of ultrasoft and PAW; it carries
  norm-conserving pseudopotentials for some elements too, which is why
  the library as a whole is what koopmans cannot take.

Relabel the fixture family that stood in for an uncut SG15 install: it
was `SG15/1.2/PBE/FR`, a label the archive does not publish.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`docstr-coverage` failed the documentation job on one missing docstring
in the new test module.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SG15/1.1/PBE/FR` installed 12 elements and `SG15/1.1/PBE/SR` 17, so a
fully relativistic run needing both Si and O had no single family to
name.

- Read each label's file selection from `_SG15_VARIANTS`, which now maps
  a version and relativistic variant to the revisions it installs: 1.1
  lists 1.0 and 1.1, and the later file wins element by element.
- Keep each pseudopotential's archive filename rather than renaming it
  to `<element>.upf`, so a family says which revision each file is.
- Restate the SG15 notes `koopmans pseudos` prints: 1.1 is composed, and
  `SG15/1.1/PBE/FR` is the fully relativistic label to name at 64
  elements.
- Move the synthetic SG15 tarball into `tests/fixtures.py`, carrying one
  member per element, revision and relativistic variant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The codecov patch check reported thirteen uncovered lines, all on the
installer's error paths and the installed-label query.

- Exercise each refusal: a non-PBE functional, an unpackaged version, a
  variant the archive lacks (asserting no bytes move first), a tampered
  checksum, an archive with no matching members, and an unrecognized
  label format.
- Cover `ensure_pseudo_family_installed`'s fall-through to the
  installer, and the installed-label query against a test profile.
- Give the synthetic archive a directory entry and a README, which the
  member walk must step over as the real tarball's are.
- List a 1.1 member ahead of its 1.0 sibling, so overlay precedence is
  pinned to the revision rather than to tarball order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@elinsc-bot
elinsc-bot marked this pull request as ready for review August 10, 2026 11:45
@elinsc-bot
elinsc-bot requested a review from elinscott August 10, 2026 11:45
elinsc-bot and others added 2 commits August 10, 2026 13:54
#109 rewrote the unrecognized-label message, the `pseudo_library` help
text and the fixture docstrings this branch also touches.

- Keep #109's message: what was checked first, then the self-install
  route and the download route. Its closing list of label grammars named
  SSSP, which this branch refuses, and ended in `/format`, which this
  branch accepts only as `upf`; both are replaced by a pointer to
  `koopmans pseudos`.
- Same substitution in the `pseudo_library` help text.
- Take #109's `_install_fake_family` docstring wholesale, and its
  wording for the uncut-family fixture over this branch's label for it.
- Update the two tests asserting the label grammar.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The installation docs offered SSSP as a family koopmans downloads, and
listed one example label per library as the way to find one. Naming an
SSSP family raises, and the labels are now enumerable.

- Replace the per-library example list with `koopmans pseudos`, keeping
  two labels to show their shape.
- Say that every family offered is norm-conserving and in UPF format,
  and that libraries mixing in ultrasoft or PAW pseudopotentials are
  refused; keep the SSSP link as the example of one.
- Close the listing with the same cutoff advice the docs and the
  unrecognized-label message give for a self-installed family.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/koopmans/aiida/setup/pseudos.py Outdated
Comment on lines +161 to +167
elif parts[0] == "SSSP":
raise ValueError(
f"'{pseudo_family}' is an SSSP family. SSSP mixes ultrasoft, PAW and "
"norm-conserving pseudopotentials, and Koopmans functionals are defined "
"for norm-conserving ones. Name a PseudoDojo or SG15 family instead; run "
"`koopmans pseudos` for the full list."
)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do a more systematic check here? We could check the pseudo headers to see if they are ultrasoft/paw using upf-tools?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or some in-built AiiDA functionality?

@elinsc-bot elinsc-bot Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — dropped the dependency. Nothing inbuilt reads the field, though: aiida-pseudo has no notion of NC/US/PAW at all (its UpfData parses the element and z_valence), and aiida-core's parse_upf gets the version and element. So it is ~40 lines here instead, reading the v2 attributes and falling back to the v1 fixed-format block.

That turned out better than upf-tools rather than merely lighter. Its v2 reader is an XML parser, and every PSlibrary pseudopotential that embeds its generation input has a Fortran namelist in PP_INFO whose bare & makes the file invalid XML — it raises ParseError on three of aiida-core's own ultrasoft test fixtures, which under a "cannot tell, so accept" rule would have let real ultrasoft pseudopotentials straight through. Scanning the header alone does not care about the rest of the file.

Validated against 75 real pseudopotentials — the SG15 tarball, aiida-core's fixtures, and a few from the QE library: all 6 non-norm-conserving refused, all 69 norm-conserving accepted. The real files also settled details a synthetic fixture would not have: PSlibrary writes is_ultrasoft="true" where SG15 writes "F", a PAW file raises the ultrasoft flag too, and its pseudo_type is USPP rather than US. Those headers are now test fixtures. 4e8393c.

Comment thread src/koopmans/aiida/setup/pseudos.py Outdated
traceback=False,
)

family.set_default_stringency("normal")

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want this hard-coded?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked, and it turns out to be load-bearing rather than decorative. install_pseudo_dojo sets low, normal and high in that order, and set_cutoffs makes a stringency the default only while it is the family's only one — so dropping this call leaves every family on low, quietly.

"normal" is not our invention either: aiida-pseudo's own install pseudo-dojo command defaults to it and calls set_default_stringency right after the same function we call. So this reproduces upstream's behaviour rather than overriding it.

That leaves no schema question — a run wanting high states calculator_parameters.ecutwfc, which takes precedence over any recommendation. Comment added saying all this; the call stays. d25a3b2.

Comment thread src/koopmans/aiida/setup/pseudos.py Outdated
Comment on lines +252 to +257
# SG15 ONCV is published as a single frozen tarball on quantum-simulation.org,
# one flat directory of ``<element>_ONCV_PBE[_FR]-<version>.upf`` files; the
# label's version/relativistic parts select which of them to install. There is
# no upstream ``aiida-pseudo`` installer for SG15, so we build the family
# ourselves. SG15 publishes no recommended cutoffs, so it is a plain
# ``PseudoPotentialFamily`` and ``ecutwfc`` comes from the input file.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worthwhile splitting pseudos.py to a module with pseudos.pseudodojo and pseudos.sg15?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. pseudos/ is now a package: _pseudodojo.py and _sg15.py each expose available_labels, install and (for SG15) NOTES, and __init__.py keeps every public name and dispatches through a registry keyed on the label's first segment — adding a library touches no shared code. The move is its own commit with no content change; the norm-conserving check went into a third module alongside them. 2d031e1.

elinsc-bot and others added 2 commits August 10, 2026 14:42
The module held two independent installers and the plumbing they share.

- `_pseudodojo.py` and `_sg15.py` each expose `available_labels`,
  `install` and, for SG15, `NOTES`; their private names lose the
  library prefix the module name now carries.
- `__init__.py` keeps every public name and dispatches through a
  registry keyed by the label's first segment, so adding a library
  touches no shared code.
- Repoint the tests that patch the archive checksum and the variants
  table at the module that reads them.

No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`set_default_stringency("normal")` reads as an arbitrary hard-coding.

- Say what it is holding off: `install_pseudo_dojo` sets low, normal and
  high in that order, and `set_cutoffs` promotes a stringency to default
  only while it is the family's only one, so dropping the call leaves
  every family on `low`.
- Name "normal" as aiida-pseudo's own default for the same install, and
  `calculator_parameters.ecutwfc` as the way past it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@elinsc-bot
elinsc-bot requested a review from elinscott August 10, 2026 13:04
A family the user installed themselves carries whatever label they gave
it, so nothing stopped an ultrasoft or PAW directory being named as
`pseudo_library` and run to completion.

- Read each pseudopotential's own UPF header, in both the v2 attribute
  layout and the v1 fixed-format block. Neither aiida-pseudo nor
  aiida-core parses the field: `UpfData` reads the element and
  z_valence, `parse_upf` the version and element.
- Refuse on positive evidence alone: `is_paw`, `is_ultrasoft`, or a
  `pseudo_type` naming a kind koopmans cannot use. A header that says
  nothing, or that cannot be read, never blocks a run.
- Check the pseudopotentials the structure will use, alongside the
  cutoff check on all three routes that reach pw steps.
- Name the offending kinds and their type in the message, and point at
  `workflow.pseudo_library`.
- Carry header excerpts from real pseudopotentials as fixtures: the
  boolean spellings differ between generators, a PAW file raises the
  ultrasoft flag as well as its own, and a PP_INFO holding a Fortran
  namelist is not well-formed XML.

The SSSP refusal stays as it is: it fires at label-parse time, before
anything is downloaded, where there are no headers to read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@elinsc-bot
elinsc-bot merged commit d4cf240 into main Aug 10, 2026
14 checks passed
@elinsc-bot
elinsc-bot deleted the k2-pseudos-command branch August 10, 2026 16:05
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.

2 participants