You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A deep review of the VPMDK v0.5.0 server/client implementation identified 17 low-priority issues, compatibility differences, and validation gaps that were intentionally deferred because they were not release-blocking.
This issue tracks those items in one place. Items marked as compatibility decisions may require an explicit project policy before implementation. Backend-dependent items should not be changed until they can be reproduced with the real upstream package.
Affected baseline: VPMDK v0.5.0 and main as of 2026-08-07.
Confirmed implementation defects
Close readiness-pipe descriptors when _daemonize() fails before descriptor ownership is transferred
src/vpmdk_core/server.py::_daemonize() creates a readiness pipe before moving its descriptors and performing the first fork. If os.fork() fails, or descriptor relocation fails because of descriptor exhaustion, the currently owned pipe descriptors can escape without being closed. Repeated failures from a long-lived in-process caller leak two descriptors per attempt.
Track the descriptors currently owned by _daemonize() and close them on every exception before the fork branches assume ownership. Successful parent and child behavior must remain unchanged.
Convert deterministic local MODEL path errors into clean one-line input diagnostics
_build_workdir_calculator() currently converts only ValueError and FileNotFoundError into WorkdirInputError. A directory used as MODEL, an unreadable file, or a path containing a non-directory component can therefore produce a backend traceback in one-shot mode, while missing files and resident-server startup receive clean diagnostics.
Extend the catch to an explicit set of deterministic local-path errors such as IsADirectoryError, PermissionError, and NotADirectoryError. Do not catch OSError broadly because network/model-registry errors such as URLError are also OSError subclasses and should remain environment failures.
Emit the GRACE ignored-DEVICE warning at the same point as one-shot execution
A resident GRACE server currently synthesizes the ignored-DEVICE warning before calling run_workdir(). This causes two observable differences:
The warning precedes the usual Note: and unknown-tag output instead of appearing at the one-shot builder position.
It is emitted even when request input processing fails before a one-shot calculator builder would be reached, for example when INCAR cannot be read.
Move the synthesized warning into the calculator-supplied run_workdir() path after successful input parsing, while preserving the shared warning text and one-shot output order.
Reject a blank DEVICE explicitly for MACE
Real MACECalculator(device="") construction fails in torch.load(map_location=""); a blank value is neither CPU nor autodetection for this backend. Server identity normalization currently treats most unclassified blank devices as autodetection.
Reject DEVICE = for MACE with a clean configuration error in both one-shot and resident-server paths, and add a real or accurately stubbed regression test for identity normalization.
Compatibility decisions
These items may change scientific results or established output contracts. They should be resolved deliberately rather than treated as mechanical bug fixes.
Decide whether relaxation output should contain at most NSW or NSW + 1 ionic frames
ASE optimizers count position updates, while attached observers also run for the initial structure. Consequently, a relaxation can write up to NSW + 1 OSZICAR F= lines, OUTCAR steps, and vasprun.xml calculations. VASP counts the initial structure as ionic step 1 and writes at most NSW.
Decide whether to preserve the existing VPMDK behavior or align it with VASP. If changed, update all output formats and regression tests together.
Align selective-dynamics semantics for non-orthogonal cells
pymatgen/AseAtomsAdaptor maps a POSCAR mask such as T T F to ASE FixCartesian. For Direct coordinates, VASP applies the mask to the corresponding lattice-coordinate components. These interpretations agree for orthogonal cells but can constrain different motion in a skewed cell.
Decide whether VPMDK should translate Direct-coordinate masks into lattice-coordinate constraints such as FixScaled. Add a non-orthogonal-cell regression test before changing existing behavior.
Decide how to handle an INCAR value placed on the following line
For input such as:
NSW =
50
pymatgen reads NSW=50, while VASP treats the assignment as empty/defaulted. VPMDK currently follows the pymatgen result.
Choose whether to preserve parser compatibility, reject the ambiguous spelling, or emulate VASP. The input must not silently mean different things without a documented policy.
Decide whether to support POSCAR/CONTCAR velocity blocks
CLI molecular dynamics always initializes velocities from TEBEG; a velocity block in POSCAR or CONTCAR is not read. This prevents true MD continuation from recorded velocities. The limitation is documented, so implementing it is a feature and compatibility change rather than a diagnostic fix.
Determine whether and how to map VASP SMASS to an ASE Nose-Hoover parameter
VPMDK interprets positive SMASS as a damping time in femtoseconds, whereas VASP defines it as a Nose mass. The values are not interchangeable. Current behavior is documented and warns for excessively strong coupling.
A change requires a verified VASP definition, a conversion rule, migration guidance, and tests demonstrating the intended ensemble behavior.
Reconsider the SMASS < 0 fallback rule
With no explicit MDALGO, VPMDK promotes negative SMASS to Langevin dynamics and treats abs(SMASS) as friction. In VASP, values such as SMASS=-3 select microcanonical/NVE behavior.
This difference is currently documented and covered by regression tests. Aligning it with VASP would be an intentional breaking behavior change and should update documentation, warnings, and tests together.
Diagnostics, interoperability, and test infrastructure
Teach the test Poscar stub to distinguish Cartesian and Direct coordinates
The tests/conftest.py stub treats Cartesian coordinate blocks as fractional coordinates because it does not parse the coordinate-mode line. Production parsing with real pymatgen is unaffected, but in-process tests can obtain misleading geometry.
Improve the stub without altering existing Direct fixtures, or replace relevant parser tests with a narrowly scoped real-pymatgen harness.
Decide whether generated OUTCAR files should be readable by ase.io.read
ASE currently raises ase.io.ParseError because generated OUTCAR files do not contain the ion_types/POTCAR header information its parser requires. The failure is explicit rather than silently returning incorrect data.
Either add the minimum compatible header data and round-trip tests or explicitly document that OUTCAR is intended for selected VASP-style consumers but not ASE parsing.
Warn once when a stress-capable backend fails to return stress at runtime
VPMDK warns when a backend is statically declared with stress=False, but if a backend declares stress support and omits stress from an actual result, the value is silently absent.
Consider a recorder-level warning emitted once per calculation. Avoid per-step warnings that would flood MD or relaxation output.
Reject an energy-only MatRIS resident configuration at startup
A resident started with MATRIS_TASK=e starts successfully, but every supported calculation request subsequently fails with input exit code 1 because VPMDK requires forces.
Reject this configuration during server startup with a clear explanation, unless a supported energy-only request mode is introduced.
Improve early diagnostics for missing charge-density model configuration
With WRITE_CHGCAR=1, missing charge-model configuration may only become apparent after the force/energy calculation has completed. A simple unconditional preflight would break supported extension and test patterns that replace predict_charge_density.
Find a runner-level or capability-aware way to surface the existing model error earlier and more clearly without rejecting injected/custom charge-density implementations.
Backend-dependent validation
Verify blank DEVICE and UPET_NEIGHBORLIST_DEVICE behavior with a real UPET installation
The behavior of UPETCalculator(device="") could not be established in the review environment. Confirm whether blank means autodetection, CPU, or an invalid value, then align resident identity normalization and neighbor-list device handling with the real builder.
Verify blank DEVICE behavior with a working ORB installation
The server currently treats blank ORB DEVICE as autodetection, while orb-models may reject get_device(""). The available orb-models installation lacked the required calculator module, so a valid one-shot/server comparison could not be performed.
Reproduce with a supported upstream version before changing normalization or validation.
Completion criteria
Every accepted behavior change has a focused regression test.
One-shot and resident-server diagnostics remain consistent where the same input reaches the same processing stage.
Backend-device normalization matches the actual upstream calculator rather than an assumed generic policy.
Scientific compatibility changes are documented as intentional and include migration notes where existing results can change.
Items that are intentionally retained as documented limitations are explicitly resolved as such.
Summary
A deep review of the VPMDK v0.5.0 server/client implementation identified 17 low-priority issues, compatibility differences, and validation gaps that were intentionally deferred because they were not release-blocking.
This issue tracks those items in one place. Items marked as compatibility decisions may require an explicit project policy before implementation. Backend-dependent items should not be changed until they can be reproduced with the real upstream package.
Affected baseline: VPMDK v0.5.0 and
mainas of 2026-08-07.Confirmed implementation defects
Close readiness-pipe descriptors when
_daemonize()fails before descriptor ownership is transferredsrc/vpmdk_core/server.py::_daemonize()creates a readiness pipe before moving its descriptors and performing the first fork. Ifos.fork()fails, or descriptor relocation fails because of descriptor exhaustion, the currently owned pipe descriptors can escape without being closed. Repeated failures from a long-lived in-process caller leak two descriptors per attempt.Track the descriptors currently owned by
_daemonize()and close them on every exception before the fork branches assume ownership. Successful parent and child behavior must remain unchanged.Convert deterministic local
MODELpath errors into clean one-line input diagnostics_build_workdir_calculator()currently converts onlyValueErrorandFileNotFoundErrorintoWorkdirInputError. A directory used asMODEL, an unreadable file, or a path containing a non-directory component can therefore produce a backend traceback in one-shot mode, while missing files and resident-server startup receive clean diagnostics.Extend the catch to an explicit set of deterministic local-path errors such as
IsADirectoryError,PermissionError, andNotADirectoryError. Do not catchOSErrorbroadly because network/model-registry errors such asURLErrorare alsoOSErrorsubclasses and should remain environment failures.Emit the GRACE ignored-
DEVICEwarning at the same point as one-shot executionA resident GRACE server currently synthesizes the ignored-
DEVICEwarning before callingrun_workdir(). This causes two observable differences:Note:and unknown-tag output instead of appearing at the one-shot builder position.Move the synthesized warning into the calculator-supplied
run_workdir()path after successful input parsing, while preserving the shared warning text and one-shot output order.Reject a blank
DEVICEexplicitly for MACEReal
MACECalculator(device="")construction fails intorch.load(map_location=""); a blank value is neither CPU nor autodetection for this backend. Server identity normalization currently treats most unclassified blank devices as autodetection.Reject
DEVICE =for MACE with a clean configuration error in both one-shot and resident-server paths, and add a real or accurately stubbed regression test for identity normalization.Compatibility decisions
These items may change scientific results or established output contracts. They should be resolved deliberately rather than treated as mechanical bug fixes.
Decide whether relaxation output should contain at most
NSWorNSW + 1ionic framesASE optimizers count position updates, while attached observers also run for the initial structure. Consequently, a relaxation can write up to
NSW + 1OSZICARF=lines, OUTCAR steps, andvasprun.xmlcalculations. VASP counts the initial structure as ionic step 1 and writes at mostNSW.Decide whether to preserve the existing VPMDK behavior or align it with VASP. If changed, update all output formats and regression tests together.
Align selective-dynamics semantics for non-orthogonal cells
pymatgen/AseAtomsAdaptormaps a POSCAR mask such asT T Fto ASEFixCartesian. For Direct coordinates, VASP applies the mask to the corresponding lattice-coordinate components. These interpretations agree for orthogonal cells but can constrain different motion in a skewed cell.Decide whether VPMDK should translate Direct-coordinate masks into lattice-coordinate constraints such as
FixScaled. Add a non-orthogonal-cell regression test before changing existing behavior.Decide how to handle an INCAR value placed on the following line
For input such as:
pymatgenreadsNSW=50, while VASP treats the assignment as empty/defaulted. VPMDK currently follows thepymatgenresult.Choose whether to preserve parser compatibility, reject the ambiguous spelling, or emulate VASP. The input must not silently mean different things without a documented policy.
Decide whether to support POSCAR/CONTCAR velocity blocks
CLI molecular dynamics always initializes velocities from
TEBEG; a velocity block in POSCAR or CONTCAR is not read. This prevents true MD continuation from recorded velocities. The limitation is documented, so implementing it is a feature and compatibility change rather than a diagnostic fix.Determine whether and how to map VASP
SMASSto an ASE Nose-Hoover parameterVPMDK interprets positive
SMASSas a damping time in femtoseconds, whereas VASP defines it as a Nose mass. The values are not interchangeable. Current behavior is documented and warns for excessively strong coupling.A change requires a verified VASP definition, a conversion rule, migration guidance, and tests demonstrating the intended ensemble behavior.
Reconsider the
SMASS < 0fallback ruleWith no explicit
MDALGO, VPMDK promotes negativeSMASSto Langevin dynamics and treatsabs(SMASS)as friction. In VASP, values such asSMASS=-3select microcanonical/NVE behavior.This difference is currently documented and covered by regression tests. Aligning it with VASP would be an intentional breaking behavior change and should update documentation, warnings, and tests together.
Diagnostics, interoperability, and test infrastructure
Teach the test
Poscarstub to distinguish Cartesian and Direct coordinatesThe
tests/conftest.pystub treats Cartesian coordinate blocks as fractional coordinates because it does not parse the coordinate-mode line. Production parsing with realpymatgenis unaffected, but in-process tests can obtain misleading geometry.Improve the stub without altering existing Direct fixtures, or replace relevant parser tests with a narrowly scoped real-
pymatgenharness.Decide whether generated OUTCAR files should be readable by
ase.io.readASE currently raises
ase.io.ParseErrorbecause generated OUTCAR files do not contain theion_types/POTCAR header information its parser requires. The failure is explicit rather than silently returning incorrect data.Either add the minimum compatible header data and round-trip tests or explicitly document that OUTCAR is intended for selected VASP-style consumers but not ASE parsing.
Warn once when a stress-capable backend fails to return stress at runtime
VPMDK warns when a backend is statically declared with
stress=False, but if a backend declares stress support and omits stress from an actual result, the value is silently absent.Consider a recorder-level warning emitted once per calculation. Avoid per-step warnings that would flood MD or relaxation output.
Reject an energy-only MatRIS resident configuration at startup
A resident started with
MATRIS_TASK=estarts successfully, but every supported calculation request subsequently fails with input exit code 1 because VPMDK requires forces.Reject this configuration during server startup with a clear explanation, unless a supported energy-only request mode is introduced.
Improve early diagnostics for missing charge-density model configuration
With
WRITE_CHGCAR=1, missing charge-model configuration may only become apparent after the force/energy calculation has completed. A simple unconditional preflight would break supported extension and test patterns that replacepredict_charge_density.Find a runner-level or capability-aware way to surface the existing model error earlier and more clearly without rejecting injected/custom charge-density implementations.
Backend-dependent validation
Verify blank
DEVICEandUPET_NEIGHBORLIST_DEVICEbehavior with a real UPET installationThe behavior of
UPETCalculator(device="")could not be established in the review environment. Confirm whether blank means autodetection, CPU, or an invalid value, then align resident identity normalization and neighbor-list device handling with the real builder.Verify blank
DEVICEbehavior with a working ORB installationThe server currently treats blank ORB
DEVICEas autodetection, whileorb-modelsmay rejectget_device(""). The availableorb-modelsinstallation lacked the required calculator module, so a valid one-shot/server comparison could not be performed.Reproduce with a supported upstream version before changing normalization or validation.
Completion criteria