deepmodeling / deepmodeling/deepmd-kit

[Code scan] Symmetrize ASE calculator stress after reshaping virial

Open
#5,689 2 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Python
Stars
2k
Forks
649
Avg merge
6d 18h
Merged PRs (30d)
15

Description

This issue comes from a Codex global scan of `deepmodeling/deepmd-kit` at commit `73de44b1f94471b2e3bdb6b11f57b34d7bc791bb`.

## Problem

The ASE calculator stores the model virial as a 3x3 matrix:

https://github.com/deepmodeling/deepmd-kit/blob/73de44b1f94471b2e3bdb6b11f57b34d7bc791bb/deepmd/calculator.py#L151-L155

but when converting virial to stress it operates on the original flat 9-vector `v[0]`:

https://github.com/deepmodeling/deepmd-kit/blob/73de44b1f94471b2e3bdb6b11f57b34d7bc791bb/deepmd/calculator.py#L157-L163

For a one-dimensional NumPy array, `.T` is a no-op. Therefore `0.5 * (v[0] + v[0].T)` does not symmetrize the virial tensor; it just returns the flat virial vector. The subsequent Voigt indexing then uses unsymmetrized off-diagonal entries.

## Impact

ASE stress returned by the DeePMD calculator can be wrong for models/inputs whose virial tensor is not exactly symmetric. This affects workflows that consume ASE stress, including cell/lattice relaxation.

## Suggested fix

Reshape before symmetrizing:

```python
virial = v[0].reshape(3, 3)
stress = -0.5 * (virial + virial.T) / atoms.get_volume()
self.results["stress"] = stress.flat[[0, 4, 8, 5, 2, 1]]
```

Alternatively, reuse `self.results["virial"]` after assigning it.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.