search

ACEsuit / mace

MACE - Fast and accurate machine learning interatomic potentials with higher order equivariant message passing.
Other
412 stars 155 forks source link

bugfix for stress loss #438

Closed JPDarby closed 4 weeks ago

JPDarby commented 4 weeks ago

Removed division of stress by n_atoms in loss to resolve #437

bernstei commented 4 weeks ago

I noticed that the train.py error report says stress_per_atom, which makes me worry - stress is intensive, it shouldn't be "per atom" the way I usually use that phrase. As I poke around the code I see delta_stress and delta_stress_per_atom, e.g. https://github.com/ACEsuit/mace/blob/dee204f1f9d587f28fd792fdad1f45039ef71e94/mace/tools/train.py#L422

Can someone (@ilyes319 ?) confirm that this kind of bug isn't present elsewhere in the code?

I'm hoping that those variables mean "total stress" and "atomic stress", rather than per_atom meaning divided by n_atoms. However, even if it is, depending on what exactly the atomic stress is (i.e. dE_site / dVol_total, or dE_site / dVol_site (= V_total / n_atoms?), it may or may not be what people are expecting.