Skip to content

Use PQAnalysis for shared parsing - #46

Merged
galjos merged 1 commit into
mainfrom
use-pqanalysis-parsers
Jun 24, 2026
Merged

Use PQAnalysis for shared parsing#46
galjos merged 1 commit into
mainfrom
use-pqanalysis-parsers

Conversation

@galjos

@galjos galjos commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

  • Parse XYZ and GEN coordinate data through PQAnalysis helpers.
  • Reuse PQAnalysis atom metadata and AtomicSystem mass/center-of-mass calculations.
  • Use the optional PQAnalysis Hessian reader when available, with a NumPy fallback and size guard.
  • Harden vibrational frequency parsing and temp-directory test fixtures.

Validation

  • python -m pytest -q
  • python -m pytest --cov=ThermoScreening --cov-report=term-missing -q
  • python -m pylint ThermoScreening
  • git diff --check

@galjos
galjos force-pushed the use-pqanalysis-parsers branch from deb3a1d to fdcc245 Compare June 23, 2026 07:34
@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.61%. Comparing base (445d9ac) to head (fdcc245).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
ThermoScreening/thermo/api.py 94.11% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #46      +/-   ##
==========================================
- Coverage   93.78%   93.61%   -0.17%     
==========================================
  Files          21       21              
  Lines        1190     1191       +1     
==========================================
- Hits         1116     1115       -1     
- Misses         74       76       +2     
Flag Coverage Δ
unittests 93.61% <96.42%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@galjos
galjos marked this pull request as ready for review June 24, 2026 12:23
@galjos
galjos merged commit 505074e into main Jun 24, 2026
3 checks passed
@galjos
galjos deleted the use-pqanalysis-parsers branch June 24, 2026 12:23
galjos added a commit that referenced this pull request Jun 25, 2026
Follow-up to #46 (shared parsing via PQAnalysis). Verifying that
refactor surfaced one critical regression plus a couple of
lower-severity issues, fixed here with regression coverage.

## Critical: Hessian reading was broken on real DFTB+ output
DFTB+ writes `hessian.out` as a flat stream of second derivatives
wrapped at a fixed number of values per line (with a ragged final line
per matrix row), not as a square grid. The refactor routed it through a
reader that requires an already-square `numpy.loadtxt` result, so it
raised on every real file:
- uniform-width output loaded as `(n, 4)` and failed the square check;
- ragged output made `numpy.loadtxt` itself fail.

This is invisible to CI because the integration tests need the
`dftb+`/`modes` binaries and are skipped, while the unit test fed a
synthetic square grid DFTB+ never emits. Restored flat-token parsing
that reads every value and reshapes to `(3N, 3N)`, keeping the explicit
size validation. Verified bit-for-bit against real 72x72 and 78x78
Hessians.

## xyz coordinate precision
`read_xyz` routed coordinates through `XYZFrameReader`, which stores
positions as float32, perturbing every value by ~1e-7 relative to the
source and to `read_gen` (float64). Now reads the coordinate columns in
float64 while keeping `XYZFrameReader` for validation, atom count,
symbols and cell detection.

## Smaller fixes
- `read_xyz` no longer masks a missing file as "Invalid XYZ coordinate
file."; it raises `FileNotFoundError` again.
- `Modes.read` stops at a trailing tag section (e.g. `saved_modes
:integer:..`) instead of casting the section header to float and
crashing.

## Test coverage
- Regression test for the Hessian reader using the authentic ragged
wrapped layout (asserts `numpy.loadtxt` cannot parse it, so the fixture
can't be reduced to a square grid).
- Small fixtures (`tests/data/calculator/modes/`) plus an integration
test that drives `Hessian.read` -> `Modes` -> wave numbers end to end
with only the `modes` binary mocked, closing the no-binary gap.
- float64-precision and missing-file tests for `read_xyz`; a
trailing-section test for `Modes.read`.

134 passed, 5 skipped (the 5 still need the DFTB+ binaries); pylint
7.94/10.
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.

1 participant