Skip to content

fix: exclude VASP data generator from pytest #387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
njzjz-bot wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: exclude VASP data generator from pytest #387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions tests/fp/data.vasp.kp.gf/make_kp_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env python3
"""Regenerate randomized VASP k-point test data on explicit invocation."""

import os

import dpdata
import numpy as np
from ase.geometry import (
cellpar_to_cell,
)


def make_one(out_dir):
"""Generate one randomized POSCAR fixture in *out_dir*."""
# [0.5, 1)
[aa, bb, cc] = np.random.random(3) * 0.5 + 0.5
# [1, 179)
[alpha, beta, gamma] = np.random.random(3) * (178 / 180) + 1

@wanghan-iapcm wanghan-iapcm Sep 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Carried over unchanged from the old file, so not something this PR introduced, and I am not asking for it here. Recording it because the file is being rewritten and the comment is false.

178 / 180 is about 0.9889, so this yields angles in [1, 1.99) degrees, not [1, 179). cellpar_to_cell takes degrees, so every generated cell is nearly collinear. I measured all 90 angles across the 30 committed fixtures: 1.0007 to 1.9848 degrees. That is also why test.000/kp.ref reads 3012 1844 2485 -- k-point counts in the thousands only happen for cells this flat at kspacing 0.16. So test_make_kp has effectively no coverage of the ordinary-cell regime.

If anyone fixes this: * 178 + 1 is not the answer. Uniformly random angle triples are usually not geometrically realizable, and 143 of 200 samples I tried raised ase's cz_sqr >= 0 assertion. It needs rejection sampling, plus regenerating kp.ref.

cell = cellpar_to_cell([aa, bb, cc, alpha, beta, gamma])
system = dpdata.System("POSCAR")

@wanghan-iapcm wanghan-iapcm Sep 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not blocking, but worth fixing while the file is open: this reads a bare relative "POSCAR", and main writes bare relative test.NNN/ directories, so the newly advertised entry point only works when the current directory is exactly tests/fp/data.vasp.kp.gf. From the repository root it exits 1 with FileNotFoundError: 'POSCAR' -- the same error that made pytest collection fail in the first place, since pytest imports collected modules with the working directory at the rootdir.

Anchoring both paths to Path(__file__).parent makes the __main__ guard robust instead of dependent on where it is invoked from.

system["cells"][0] = cell
os.makedirs(out_dir, exist_ok=True)
system.to_vasp_poscar(os.path.join(out_dir, "POSCAR"))


def main(ntest=30):
"""Regenerate all randomized fixture directories."""

@wanghan-iapcm wanghan-iapcm Sep 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This docstring, and the module one on line 2, promise a regeneration this function does not perform. make_one writes only POSCAR, but each test.NNN/ directory also contains a kp.ref that tests/fp/test_vasp.py compares computed k-points against, and nothing in the repository writes those.

Running exactly what the docstrings advertise:

$ cd tests/fp/data.vasp.kp.gf && python make_kp_data.py     # exit 0
$ pytest tests/fp/test_vasp.py -k make_kp
FAILED tests/fp/test_vasp.py::TestVASPInputs::test_make_kp - AssertionError: False is not true

and recomputing afterwards, all 30 directories disagree with their committed kp.ref.

Before this PR the loop only ran by accident at import time, so this was a latent hazard. Adding main(), the __main__ guard and these docstrings makes it an invitation. Either write kp.ref here too (make_kspacing_kpoints(cell, 0.16, False)), or say in the docstring that the references are not regenerated and must be redone by hand.

for index in range(ntest):
make_one("test.%03d" % index)


if __name__ == "__main__":
main()
26 changes: 0 additions & 26 deletions tests/fp/data.vasp.kp.gf/make_kp_test.py
View file
Open in desktop

This file was deleted.