search

materialsvirtuallab / matgl

Graph deep learning library for materials
BSD 3-Clause "New" or "Revised" License
256 stars 59 forks source link

Ensure volume is positive in stress calculation #198

Closed PROA200 closed 10 months ago

PROA200 commented 10 months ago

Currently,

from pymatgen.util.testing import PymatgenTest
import matgl
from matgl.ext.ase import M3GNetCalculator
def LiFePO4():
    return PymatgenTest.get_structure("LiFePO4")
potential_m3gnet = matgl.load_model("M3GNet-MP-2021.2.8-PES")
m3gnet = M3GNetCalculator(potential=potential_m3gnet, stress_weight=1.)
print(m3gnet.get_stress(atoms = LiFePO4().apply_strain(0.1).to_ase_atoms()))

gives

array([[-2.4297804e+01,  2.0667364e-01, -1.2357983e-04],
       [ 2.0667414e-01, -2.3150066e+01,  4.2587479e-05],
       [-1.2232375e-04,  4.3453932e-05, -2.6601597e+01]], dtype=float32)

, which is incorrect (stress should be positive for a positive strain). This seems to be because, when calculating the stresses, volume is calculated as volume = torch.det(lattice), but for the structure above, this value is negative. After correcting this error, the negation of the above matrix is obtained, while the results for other structures are unchanged.

(Right now, the structure retrieved from PymatgenTest is rather cruel, and in reality, this works fine using structures retrieved from the MP API.)

codecov[bot] commented 10 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (f2d8ed9) 98.59% compared to head (5a65027) 98.59%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #198 +/- ## ======================================= Coverage 98.59% 98.59% ======================================= Files 28 28 Lines 1926 1926 ======================================= Hits 1899 1899 Misses 27 27 ```

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

kenko911 commented 10 months ago

Hi @PROA200, Thanks for reporting this, I had a look at the lattice matrix of the testing LiFePO4, which is [[0.000000 0.000000 -4.744800], [-0.053123 -6.065537 0.000383], [10.410370 0.000000 0.000035]]. The definition of lattice matrix is uncommon and therefore results in a negative determinant (volume). Taking an absolute value would be safe for all cases.