Skip to content

Parse VASP zero-weight (hybrid/HSE) band structures - #339

Merged
ndaelman-hu merged 11 commits into
developfrom
feat/vasp-hybrid-band-structure
Aug 18, 2026
Merged

Parse VASP zero-weight (hybrid/HSE) band structures#339
ndaelman-hu merged 11 commits into
developfrom
feat/vasp-hybrid-band-structure

Conversation

@ndaelman-hu

Copy link
Copy Markdown
Contributor

Purpose

VASP hybrid/HSE band structures are self-consistent runs written as an explicit
k-point list — a weighted SCF mesh followed by a zero-weight band path — with no
<generation> block, so sampling_method is never set. The parser stored these
as a flat eigenvalues table, so band_structure_electronic was never created
and the band structure could not be plotted (the GUI shows "no data"). This PR
detects the zero-weight tail and promotes it to a band structure.

Scope

Included

  • Detect the zero-weight k-point tail and promote it to band_structure_electronic, alongside the existing Line-path handling.
  • Parse <kpoints_labels> and segment the path by high-symmetry labels; handle discontinuous (branch-split) paths — a list-adjacent label pair is a branch break (no bridging segment), with a label-free distance-jump fallback.
  • Physical, plugin-configurable discontinuity threshold in Å⁻¹ (VASPEntryPoint.band_path_discontinuity_threshold, default 0.1), applied as a Cartesian k-distance via the reciprocal cell.
  • Fail-safe signal derivation (_band_path_signals): never computed for a Line-path run, and degrades to flat eigenvalues rather than raising on a malformed k-point record.

Out of scope

  • Code-agnostic band-structure derivation / other parsers.
  • Pure line-mode and normal-SCF paths are untouched.

Reviewer Notes

  • The zero-weight tail is the robust detector; labels drive segmentation, with the distance-jump fallback only for label-free split paths.
  • The large diff is the test fixture tests/data/vasp/Cu3PS4_hybrid_bands/vasprun.xml (~21k lines) — a trimmed real HSE run (<dos>/<projected> stripped), verified to reproduce the full-file parse.

Breaking Changes

  • None.

Testing / Validation

  • pytest tests/test_vaspparser.py — 45 pass (23 pre-existing + hybrid end-to-end + parametrized segmentation/signal unit tests).
  • End-to-end: parser -> BandStructureNormalizer yields a ~1.6 eV gap on Cu3PS4 where previously there was no band structure at all.

Self-consistent hybrid band runs write an explicit k-point list (weighted
SCF mesh followed by a zero-weight band path) and carry no `<generation>`
block, so `sampling_method` is never set. The parser previously stored such
runs as a flat `eigenvalues` table, so no `band_structure_electronic` was
created and the band structure could not be plotted.

- Detect the zero-weight tail and promote it to `band_structure_electronic`,
  in a new branch alongside the existing `Line-path` handling.
- Parse the `<kpoints_labels>` block into `kpoints_info['labels']` as ordered
  `(label, index)` pairs and use them to split the path into segments; fall
  back to a single continuous segment when labels are absent.
- Add `_clean_kpoint_label` to normalize labels (e.g. `\Gamma` -> `Γ`).

Add an end-to-end test on a trimmed `Cu3PS4` HSE `vasprun.xml` fixture
covering the promoted band structure and its seven labelled segments.
A k-path may be split into disconnected branches (e.g. `Γ X M Γ | R A`); a
branch break must yield separate segments with no bridging line across the
gap. The previous logic built a segment between every consecutive label pair,
which would draw a spurious segment across such a discontinuity.

- Extract segmentation into a pure `_split_band_path` helper. With labels, a
  pair of labels adjacent in the k-point list (no interior samples) is treated
  as a branch break and yields no segment. Without labels, the path is split
  where the step between consecutive k-points exceeds a factor of the median
  step (a discontinuity); otherwise a single segment is emitted.
- Switch segment slicing to fancy indexing, removing the assumption that the
  zero-weight points are contiguous in the full k-point list.
- Log when discontinuities are inferred heuristically from k-point spacing.

Add parametrized unit tests for `_split_band_path` covering continuous,
distance-split, non-adjacent revisit (must not split), labelled branch-break,
and labelled continuous paths.
Replace the median-relative (dynamic) discontinuity threshold with a static
distance threshold in fractional reciprocal coordinates, exposed through the
plugin mechanism as `VASPEntryPoint.band_path_discontinuity_threshold`
(default 0.25). The parser resolves it from the `parsers/vasp` entry point via
`config.get_plugin_entry_point`, falling back to a module-level default when
config is unavailable (e.g. standalone parsing).

`_split_band_path` now breaks a label-free path where the step between
consecutive k-points exceeds the configured threshold, instead of a multiple
of the median step.
The label-free discontinuity threshold previously measured steps in fractional
reciprocal coordinates, which is not a true distance for non-orthogonal cells
and is not comparable across materials. Convert consecutive-k-point steps to a
Cartesian k-distance using the reciprocal lattice (2π convention) derived from
the parsed cell, so the threshold is a physical value in inverse angstrom.

- `_split_band_path` accepts a `reciprocal_cell` and measures the step in Å⁻¹
  when it is given.
- `parse_eigenvalues` builds the reciprocal cell from `system_ref` lattice
  vectors and passes it through.
- Default `band_path_discontinuity_threshold` set to 0.1 Å⁻¹ (well above typical
  interior sampling, below high-symmetry-point jumps).

Extend the `_split_band_path` unit tests with cases verifying a step crosses the
threshold only after reciprocal-cell scaling.
The zero-weight/label signals were computed before the `Line-path` branch that
does not use them, so a malformed `weights`/`labels` record could break the
`Line-path` and flat-eigenvalue paths that previously worked.

- Guard the derivation behind `sampling_method != 'Line-path'` so a Line-path
  run never computes `boundaries`/`zero_weight`/`has_zero_weight_path`.
- Wrap the derivation in a fail-safe that degrades to flat eigenvalues instead
  of raising, preserving the pre-change behaviour on malformed input.
- Clarify why the label endpoints are sorted by k-point index (the label
  extraction groups repeated labels, losing document order).
Move the zero-weight/label signal derivation into a pure `_band_path_signals`
helper so the fail-safe behaviour is unit-testable without a fixture.

Add parametrized tests covering a weighted mesh, a zero-weight tail, the
all-zero-weight edge (not a path), label sorting by k-point index, a weight
length mismatch, and malformed weights/labels degrading to (None, None, False)
instead of raising.
Both band-path branches created the `BandStructure` with per-spin `band_gap`
records and appended `BandEnergies` segments with identical code. Extract
`_new_band_structure` and `_append_band_segment` helpers and call them from
both branches. Pure refactor; segmentation logic is unchanged.
@coveralls

coveralls commented Aug 17, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 32053390790

Coverage increased (+0.01%) to 92.853%

Details

  • Coverage increased (+0.01%) from the base build.
  • Patch coverage: 6 uncovered changes across 1 file (139 of 145 lines covered, 95.86%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
electronicparsers/vasp/parser.py 105 99 94.29%
Total (3 files) 145 139 95.86%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 39066
Covered Lines: 36274
Line Coverage: 92.85%
Coverage Strength: 0.93 hits per line

💛 - Coveralls

@ndaelman-hu ndaelman-hu self-assigned this Aug 17, 2026
@ndaelman-hu
ndaelman-hu requested review from ladinesa and a lite review from Copilot August 17, 2026 17:42
@ndaelman-hu ndaelman-hu added the feature / enhancement New feature or request label Aug 17, 2026
@ndaelman-hu

Copy link
Copy Markdown
Contributor Author

A screenshot of the band structure generated locally.

Screenshot from 2026-08-17 19-19-06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the VASP parser’s handling of hybrid/HSE band-structure outputs where VASP writes an explicit k-point list consisting of a weighted SCF mesh followed by a zero-weight band path (often without a <generation> block). It promotes the zero-weight tail into band_structure_electronic so the resulting band structure can be plotted instead of remaining as a flat eigenvalues table.

Changes:

  • Add zero-weight-tail detection and segmentation (label-driven with distance-jump fallback) to construct band_structure_electronic for hybrid/HSE vasprun.xml outputs.
  • Parse <kpoints_labels> and use labels to split high-symmetry segments (including branch-break handling).
  • Introduce a plugin-configurable discontinuity threshold via a VASP-specific entry point and add end-to-end + unit tests for segmentation/signal derivation.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
electronicparsers/vasp/parser.py Adds helpers for label cleanup, signal derivation, path splitting, and extends eigenvalue parsing to build band_structure_electronic from zero-weight band paths.
electronicparsers/__init__.py Introduces VASPEntryPoint with a configurable band_path_discontinuity_threshold and wires it into the VASP entry point.
tests/test_vaspparser.py Adds an end-to-end hybrid/HSE band-structure test plus unit tests for segmentation and fail-safe signal derivation.
Suppressed comments (1)

electronicparsers/vasp/parser.py:1541

  • The <kpoints_labels> parsing converts indices with int(index) - 1 without guarding. A single malformed label index in the XML would raise and prevent parsing entirely, even though downstream logic is intended to be fail-safe. Consider ignoring malformed indices instead of raising.
                label_pairs = []
                for name, indices in labels.items():
                    indices = indices if isinstance(indices, list) else [indices]
                    for index in indices:
                        label_pairs.append((name, int(index) - 1))
                self._kpoints_info['labels'] = label_pairs

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread electronicparsers/vasp/parser.py Outdated
Comment thread electronicparsers/vasp/parser.py
Address code-review feedback that the fail-safe intent had holes:

- Filter out-of-range label indices in `_band_path_signals`; an index outside
  `[0, n_kpoints)` would pass the derivation but later raise `IndexError` when
  slicing the eigenvalues.
- Skip a malformed `<kpoints_labels>` index during parsing instead of failing
  the whole `kpoints_info` build.
- Disable distance-based splitting when no reciprocal cell is available: the
  threshold is a physical k-distance (Å⁻¹), so without a cell the path is kept
  as a single continuous segment rather than thresholding raw fractional
  coordinates in the wrong unit.

Extend the unit tests: out-of-range labels are dropped, and a label-free path
with no reciprocal cell stays continuous.
DEFAULT_BAND_PATH_DISCONTINUITY_THRESHOLD = 0.1


def _band_path_discontinuity_threshold():

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

not necessary to define as function? is there a circular import error to fix?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I moved from nomad.config import config to module scope, so it's no longer lazy.

Kept it as a function, though: the try/except is needed because when the parser is invoked directly (as in the tests) the parsers/vasp entry point isn't registered, so get_plugin_entry_point raises and we fall back to the entry point's own Field default. Injecting config via load() like the normalizers do would be cleaner, but it needs a bigger change since the base EntryPoint.load() returns a MatchingParserInterface and the parser is built later from parser_class_name, so config never reaches the constructor.

Comment thread electronicparsers/vasp/parser.py Outdated
return boundaries, zero_weight, has_zero_weight_path


def _new_band_structure(sec_scc, valence_max, conduction_min):

@ladinesa ladinesa Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This belongs to the class, actually as a local function in the class method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — moved both _new_band_structure and _append_band_segment onto VASPParser as @staticmethods. They don't use any instance state, so a static method keeps them in the class without an unused self, and avoids nesting them inside the parse_eigenvalues closure that calls them.

Comment thread electronicparsers/vasp/parser.py Outdated
return sec_k_band


def _append_band_segment(sec_k_band, kpoints, energies, occupations, labels=None):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same here — moved to a @staticmethod alongside the other builder.

Hoist the `nomad.config` import to module scope (no circular import) and drop
the duplicated `DEFAULT_BAND_PATH_DISCONTINUITY_THRESHOLD` constant.
`_band_path_discontinuity_threshold` now falls back to
`vasp_parser_entry_point.band_path_discontinuity_threshold` when the
`parsers/vasp` entry point is unregistered (e.g. direct/test invocation),
making the entry-point `Field` default the single source of truth.

Make `_split_band_path`'s `reciprocal_cell` and `threshold` required now that
the module constant no longer supplies a default; all call sites already pass
both explicitly.
Attach `_new_band_structure` and `_append_band_segment` to the class per
review feedback. They use no instance state, so `@staticmethod` keeps them in
the class namespace without an unused `self` and without deepening the nested
`parse_eigenvalues` closure that calls them.
Cover `_band_path_discontinuity_threshold`: it returns the configured value when
the `parsers/vasp` entry point resolves, and falls back to the entry-point Field
default when the lookup raises (the direct/test-invocation path). Locks the
fallback that the parser depends on but which is otherwise only exercised
implicitly.
@ndaelman-hu
ndaelman-hu merged commit eb6be0c into develop Aug 18, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature / enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants