search

stfc / janus-core

Tools for machine learnt interatomic potentials
https://stfc.github.io/janus-core/
BSD 3-Clause "New" or "Revised" License
9 stars 7 forks source link

name ML properties in the results file by architecture, energy become… #176

Closed alinelena closed 2 months ago

alinelena commented 3 months ago

…s mace_energy,...

alinelena commented 3 months ago

At the moment this seems to write out things twice:

Lattice="5.62 0.0 0.0 0.0 5.62 0.0 0.0 0.09808252417753258 5.619144046778919" 
Properties=species:S:1:pos:R:3:spacegroup_kinds:I:1:mace_mp_forces:R:3:forces:R:3
spacegroup="P 1" 
unit_cell=conventional
occupancy="_JSON {\"0\": {\"Na\": 1.0}, \"1\": {\"Cl\": 1.0}, \"2\": {\"Na\": 1.0}, \"3\": {\"Cl\": 1.0}, \"4\": {\"Na\": 1.0}, \"5\": {\"Cl\": 1.0}, \"6\": {\"Na\": 1.0}, \"7\": {\"Cl\": 1.0}}" 
mace_mp_energy=-27.020331256158105
mace_mp_stress="-0.006830042553032722 -0.006875546081961216 -0.006926724236709431 0.0012606249302715168 -1.3664263946131713e-19 3.979321394392908e-19"
energy=-27.020331256158105 
stress="-0.006830042553032722 3.979321394392908e-19 -1.3664263946131713e-19 3.979321394392908e-19 -0.006875546081961216 0.0012606249302715168 -1.3664263946131713e-19 0.0012606249302715168 -0.006926724236709431"
free_energy=-27.020331256158105
pbc="T T T"

Looks mostly ok otherwise

yes on it to fix it but is more delicate than initially thought... by time you are in london shall be done.

ElliottKasoar commented 2 months ago

It looks like you've already done the equivalent changes for the Python tests, but test_singlepoint_cli.py::test_singlepoint fails because now that we don't write the results of the calculator directly, it no longer attaches a SinglePointCalculator when the data is read back in, and so get_potential_energy etc. can't be called.

Is this definitely what we want?

Might it be preferable to have some sort of store-as-info setting, rather than the default? Otherwise, it feels hidden away a little.

It also means if we do something like single_point.run() etc. repeatedly, it'll have to explicitly recalculate everything, when it might already be stored, just not where it expects.

alinelena commented 2 months ago

just to note, this is an abuse of ase atoms object. till a proper mechanism to store user properties will be available