Allow model to request additional inputs from the engine

[GardevoirX]

Currently the model can only ask for properties like energies, forces, and stresses from the simulation engine, which largely limited the scope of calculateable properties. This PR aims at allowing for more additional inputs from ase. Will be ready for review after the unit conversion is implemented (after the metadata feature of TensorMap is released).

Closes #104

Contributor (creator of pull-request) checklist

• Tests updated (for new features and bugfixes)?
• Documentation updated (for new features)?
• Issue referenced (for PRs that solve an issue)?

Reviewer checklist

• CHANGELOG updated with public API or any other important changes?

[Luthaf]

This is going in the right direction!

[Luthaf]

For now I would limit properties that can be passed around to the standard ouputs (which we might want to rename to "standard data layout?"): https://docs.metatensor.org/metatomic/latest/outputs/index.html

[Luthaf]

Or we expose these as ase::<name>, and document them explicitly.

[GardevoirX]

Might the latter one would be better, since the same property might have different names in different softwares

[Luthaf]

Well, ideally one would just request "charge" with per_atom=True, and get the corresponding data from any simulation engine.

We can use non-standard names for things that are only available in one engine, but things should be standardized where possible.

[Luthaf]

Could you add the new outputs in a separate PR? It will also need documentation and changes to _check_output, like for example in #63.

[Luthaf]

The code also need to convert the units of the extra inputs in AtomisticModel.forward

[Luthaf]

Suggested change

	raise ValueError(f"The property {quantity} is not available in `ase`.")
	raise ValueError(f"The model requested '{quantity}', which is not available in `ase`.")

Although should we check in atoms.arrays before throwing this error? Especially for custom things

[GardevoirX]

so if we don't have a getter for the requested quantity, we try to check it in atoms.arrays? and if we fail to find it, we will throw an error?

[Luthaf]

We have three kinds of data a model could request

• standard data, as done in this PR. This is supported across models and engines
• engine-specific data, i.e. ase::<something>. Models can request it, but only ASE can provide it. The list of these is fixed and documented in the ASE calculator.
• fully custom data, i.e. whatever::name. Here we could try to find it in arrays, so someone can write a module using the data, and create ase.Atoms with the data in arrays/info, and everything just works.

[GardevoirX]

ah okay, understood

[Luthaf]

IMO these should be made available as ase::<name>, using the same syntax as for custom outputs

[GardevoirX]

hmm but if we don't give these properties getters and unit conversion rules, users still cannot have access to these, so I don't see the point of renaming them as ase::<name>

[Luthaf]

I don't understand what you mean? If someone requests ase::free_energy in kJ/mol, we can return that taking the data from ase and converting from eV to kJ/mol

[GardevoirX]

Could you add the new outputs in a separate PR? It will also need documentation and changes to _check_output, like for example in #63.

Sure, I do it after we merge this? Or it would be the perquisite of this PR?

[Luthaf]

It can be done before or after this PR, but the corresponding code should be removed from this one!

[GardevoirX]

It can be done before or after this PR, but the corresponding code should be removed from this one!

okay let me think about it, for now I think these two are coupled with each other...

[Luthaf]

You should be able to add new "standard output" as a standalone PR, even if no code is using this output yet.