dEdLat contribution for stress calculations is added and Universal Potentials are updated

kenko911 commented 10 months ago

Summary

I finally figured out that the stress contribution due to the change of lattice vectors in the Potential class is missing in the current implementation, now I have modified the implementations to add the strain (st) variable for lattices to include the gradient of potential energy with respect to lattice vectors for stress calculations. I also added unit tests for checking force and stress implementations with central finite difference methods and completed the retraining of both universal M3GNet MLIPs. The resulting pretrained universal MLIPs achieve better accuracy and stability in benchmark examples. Finally, I have checked all Jupyter Notebook examples, and everything works for the implementation.

Checklist

[x] Google format doc strings added. Check with ruff.
[x] Type annotations included. Check with mypy.
[x] Tests added for new features/fixes.
[x] If applicable, new classes/functions/modules have duecredit @due.dcite decorators to reference relevant papers by DOI (example)

Tip: Install pre-commit hooks to auto-check types and linting before every commit:

pip install -U pre-commit
pre-commit install

codecov[bot] commented 10 months ago

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (9036184) 98.00% compared to head (233cb4d) 98.59%.

Files	Patch %	Lines
matgl/utils/training.py	78.94%	4 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #189 +/- ## ========================================== + Coverage 98.00% 98.59% +0.59% ========================================== Files 28 28 Lines 1901 1928 +27 ========================================== + Hits 1863 1901 +38 + Misses 38 27 -11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

J-You commented 2 weeks ago

I'm checking out the source in VS Code, and I jumped in from this merged PR. Not sure if this is the right place to ask, but I've got a question about model/_m3gnet.py. On lines 341 and 342, when converting from fractional to Cartesian coordinates, why is it that the matrix product only involves the first row vector of the lattice?

g.edata["pbc_offshift"] = torch.matmul(g.edata["pbc_offset"], lat[0])
g.ndata["pos"] = g.ndata["frac_coords"] @ lat[0]

If so, then how does the subsequent code operate, such as the function compute_pair_vector_and_distance:

dst_pos = g.ndata["pos"][g.edges()[1]] + g.edata["pbc_offshift"]
src_pos = g.ndata["pos"][g.edges()[0]]
bond_vec = dst_pos - src_pos
bond_dist = torch.norm(bond_vec, dim=1)

Jiang You

materialsvirtuallab / matgl