Ensure `free_energy` property is `float`

materialsvirtuallab / matgl

Graph deep learning library for materials

BSD 3-Clause "New" or "Revised" License

233 stars 57 forks source link

Ensure `free_energy` property is `float` #225

Closed Andrew-S-Rosen closed 4 months ago

Andrew-S-Rosen commented 5 months ago

This is a follow-up to #203.

The Calculator.results dictionary from an M3GNet run yields something like 'free_energy': array(-4.0938973, dtype=float32) for the free energy property. This causes some havoc in places (e.g. monty jsanitization) since the array isn't iterable. It doesn't have a defined len without the brackets. We should really make this a float instead anyway. That's what I do in this PR.

codecov[bot] commented 5 months ago

Codecov Report

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

Comparison is base (c99288e) 98.86% compared to head (40956f8) 98.86%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #225 +/- ## ======================================= Coverage 98.86% 98.86% ======================================= Files 28 28 Lines 1946 1946 ======================================= Hits 1924 1924 Misses 22 22 ```

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

kenko911 commented 5 months ago

Hi @Andrew-S-Rosen, thanks for your PR! I noticed that the current AdvancedSoft MatGL-LAMMPS interface breaks due to the changes for the datatype of potential_energy() from numpy.ndarray into float (see line 105 in https://github.com/advancesoftcorp/lammps/blob/based-on-lammps_2Aug2023/potentials/M3GNET/matgl_driver.py). I will submit a pull request to AdvancedSoft to fix the bug.