Refactor unit conversion and allow additional inputs#49
Conversation
GardevoirX
force-pushed
the
units-conversion
branch
from
1ed3a33 to
65d6605
Compare
| {"lmp::density", { | ||
| {"real", ""}, | ||
| {"metal", ""}, | ||
| {"si", ""}, | ||
| {"cgs", ""}, | ||
| {"electron", ""}, | ||
| {"micro", ""}, | ||
| {"nano", ""} | ||
| }}, | ||
| {"lmp::dipole_moment", {}}, |
There was a problem hiding this comment.
because in the unit page of lammps these quantities are listed but not the standard quantities of mta
There was a problem hiding this comment.
sorry, I meant why use empty strings for lmp::density but nothing for lmp::dipole_moment? Anyway, I'd remove these for now, until we add a way to request/output these specifically
There was a problem hiding this comment.
ah I see, yes let's remove them
There was a problem hiding this comment.
no I replaced it with a newer version
https://github.com/metatensor/lammps/blob/units-conversion/examples/PACKAGES/metatomic/nickel-lj.pt
| for (const auto& [property, input]: inputs) { | ||
| const auto& property_name = property.c_str(); | ||
| const auto& unit = input->unit().c_str(); | ||
| if (strcmp(property_name, "mass") == 0 || strcmp(property_name, "masses") == 0) { |
There was a problem hiding this comment.
LAMMPS does not need to support both old and new names. It should call requested_inputs on the TorchScript model with use_new_names=true and only handle the new names.https://github.com/metatensor/metatomic/blob/7ee8113b5beb890145ff1f60db7e3525562e1959/python/metatomic_torch/metatomic/torch/model.py#L498
2 participants
Summary
This PR aims at updating the unit-conversion-related codes with the newly implemented unit parser. Meanwhile it adds supports to the additional inputs to the models.
Will be ready when the new version of metatomic releases