fix(vasp): warn when POTCAR and POSCAR species order disagree - #1046
fix(vasp): warn when POTCAR and POSCAR species order disagree#1046njzjz wants to merge 1 commit into
Conversation
VASP takes the `ions per type` counts from the POSCAR but the species identities from the POTCAR, in POTCAR order, and pairs them positionally without checking. When the two files list species in different orders VASP neither reorders nor complains, so every count lands on the wrong element and dpdata faithfully reports the mislabeled system. The OUTCAR in deepmodeling#778 shows exactly this: POTCAR order is Li O F Na Mg Al K Ca while the POSCAR title reads Li3 F39 K3 Mg3 Ca3 Na3 Al10 O6. The per-type mass array VASP printed, POMASS = 7.01 16.00 19.00 22.99 24.30 26.98 39.10 40.08 confirms VASP used the POTCAR order, so dpdata's output matches what was computed -- but the reporter had no way to see that their potentials did not match their structure. Compare the POTCAR-derived names against the POSCAR title when that title reads as a chemical formula, and warn on a mismatch. The title is free-form text, so it is only parsed when every token is an element symbol with an optional count; VASP pads it to 40 characters, so a full-width title has its final, possibly cut, token dropped. All seven OUTCAR fixtures in the test suite stay silent. Fixes deepmodeling#778. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Merging this PR will not alter performance
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1046 +/- ##
==========================================
+ Coverage 87.63% 87.67% +0.04%
==========================================
Files 90 90
Lines 9209 9242 +33
==========================================
+ Hits 8070 8103 +33
Misses 1139 1139 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesVASP species order validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant OUTCAR
participant system_info
participant order_validation
OUTCAR->>system_info: provide POSCAR title and POTCAR records
system_info->>order_validation: compare POSCAR species with atom_names
order_validation-->>system_info: return silently or emit order warning
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR adds a diagnostic warning to the VASP OUTCAR parser to help users detect a common upstream input mistake: when POTCAR species order disagrees with the species order implied by the POSCAR title, causing VASP (and therefore dpdata) to label species/counts positionally in POTCAR order without error.
Changes:
- Add
formula_from_poscar_title()andcheck_potcar_poscar_order()to compare POTCAR-derived atom names against a formula-like POSCAR title and emit a warning on mismatch. - Extend
system_info()to capture thePOSCAR = ...title from OUTCAR and invoke the new mismatch check. - Add a new test module covering formula parsing edge cases and an end-to-end mismatch warning scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
dpdata/formats/vasp/outcar.py |
Parses POSCAR = ... title and warns when POTCAR order conflicts with formula-like POSCAR title. |
tests/test_vasp_outcar_species_order.py |
Adds unit and integration tests for title parsing and POTCAR/POSCAR order mismatch warnings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # the POSCAR title echoed among the start parameters, e.g. | ||
| # POSCAR = Li3 F39 K3 Mg3 Ca3 Na3 Al10 O6 | ||
| m = re.match(r"\s*POSCAR\s*=\s?(.*)$", ii) | ||
| if m: | ||
| poscar_title = m.group(1) |
Fixes #778.
What is actually happening
I reproduced the report with the OUTCAR attached to the issue:
dpdata is not reordering anything. The OUTCAR lists its POTCAR blocks in the order
Li O F Na Mg Al K Ca, while the POSCAR title saysLi3 F39 K3 Mg3 Ca3 Na3 Al10 O6. VASP takesions per typefrom the POSCAR and the species identities from the POTCAR, in POTCAR order, and pairs them positionally without checking that the two agree.The per-type mass array VASP itself printed settles which pairing it used:
Type 2 has mass 16.00 and 39 ions, so VASP computed Li3 O39 F3 Na3 Mg3 Al3 K10 Ca6 — exactly what dpdata reports.
type.rawandtype_map.raware consistent with the calculation that was run. The real problem is upstream: that POTCAR was concatenated in a different order than the POSCAR lists its species, so the calculation used the wrong potential for seven of the eight species, and nothing told the user.Change
Since dpdata cannot fix the data, it can at least stop the silence.
system_infonow compares the POTCAR-derived names against the POSCAR title and warns when they disagree:The parsed data is unchanged — the warning is purely diagnostic.
Avoiding false positives
The POSCAR title is arbitrary user text, so it is only read as a formula when every token matches
^[A-Z][a-z]?\d*$and the symbol is indpdata.periodic_table.ELEMENTS. VASP pads the title into a 40-character field, so a full-width title is treated as truncated and its final token is dropped —Cr3 Mn5 ... Nb5 Mcompares the first ten species and ignores the cut one. A single-token title is rejected, since one species cannot disagree on ordering and a one-word title is far more likely to be prose.Checked against every OUTCAR fixture in the repo; all stay silent:
OUTCAR.ch4.1stepH C[H, C]OUTCAR.ch4.unconvergedH C[H, C]OUTCAR.Ge.vdwFile generated by python recompute scripOUTCAR.h2o.mdPOSCAR file written by OVITOOUTCAR.h2o.md.10POSCAR file written by OVITO6362_OUTCARB8 O6[B, O]outcar.longit/OUTCARCr3 Mn5 ... Nb5 MLi3 F39 K3 ...Validation
tests/test_vasp_outcar_species_order.py, 13 new cases covering formula parsing, prose rejection, unknown symbols, truncation, and an end-to-end OUTCAR whose twoTITELrecords are swapped so the POTCAR order contradicts the title.python -m unittest discover— 2249 passed, 28 skipped.ruff format/ruff checkclean.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests