Physics based priors

[peastman]

I want to implement some physics based priors. An example would be a Coulomb interaction based on pre-computed partial charges that are stored in the dataset. BasePrior.forward() is supposed to return a list of per-atom energy contributions, but physics based interactions usually do not decompose in that way. It would be much easier if it could just return a total energy for each sample.

What do you recommend as the cleanest way of implementing this?

[dav0dea]

BasePrior.forward() returns the updated atomwise energy predictions, not only the prior contribution (see the Atomref prior as an example). It would be possible to add the prior divided by the number of atoms to each atomwise prediction but I don't think this is very clean. It might be better to have an atomwise_prior flag in BasePrior which determines whether it should be applied before or after reducing the atomwise predictions. We would then apply the prior model either before (as it is done now) or after the following line, based on this flag: https://github.com/compsciencelab/torchmd-net/blob/7a92b3ef7739da14aa8cdd2d87c8e9462d86cf14/torchmdnet/models/output_modules.py#L73
The flag can then be set through the super().__init__(atomwise_prior=True/False) call of the deriving class.

Feel free to open a PR if you want to have a go at this, otherwise I can also make this change.

[peastman]

Sounds good, I'll add that.

[peastman]

Sorry for reopening an old issue - this never actually got implemented! I'm about to start on it now. I'll add the atomwise_prior flag as suggested above. I also have a few other questions about the best way of implementing particular potentials.

Physics based potentials will often require extra information. For example, one prior I want to add is the ZBL stopping potential for short range repulsion. It requires knowing the atomic number of every atom. forward() takes an argument z which often corresponds to the atomic number, but not always. For example, in some of my models I define atom types by the combination of element and formal charge, so z is an arbitrary atom type index. I need to separately provide both an atom type and an atomic number.

A more complicated example is Coulomb, which depends on the charge of each atom. That could be a fixed number (formal charge or precomputed partial charge), but in other cases it will itself be calculated as an output of the model.

What's the best way of providing extra information like this that's needed to compute a physics based potential?

[peastman]

Another question: currently it only lets you specify a single prior model, but often you may want more than one. A ZBL potential for short range repulsion, plus Coulomb for long range interactions, plus D4 for dispersion. (SpookyNet includes all of those.) Any thoughts on the best way of handling this?

[peastman]

And another question: how do we want to handle units? Nothing in the current code worries about them, except sometimes for a particular dataset. The inputs and outputs to a model are just numbers that might have any units. But physical potentials involve hardcoded constants like eps_0 whose values depend on units. How should we handle this? Some possibilities include

• Standardizing on a particular set of units for all models.
• Standardize on a particular set of units for a particular potential function, and require the user to provide scaling factors for converting positions, energies, etc. to the required units.
• Implement it at a higher level, where a model specifies what units it uses.
  • We could also make each Dataset specify its units, and conversions between the dataset and model units would happen automatically.

[giadefa]

can you use that single model as a wrapper for all the models that you would like to use? It's just the entry point.

…

On Mon, Oct 3, 2022 at 4:45 PM Peter Eastman ***@***.***> wrote: Another question: currently it only lets you specify a single prior model, but often you may want more than one. A ZBL potential for short range repulsion, plus Coulomb for long range interactions, plus D4 for dispersion. (SpookyNet includes all of those.) Any thoughts on the best way of handling this? — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3KUOQDFOZQNOJDW332PW3WBNAWFANCNFSM47GPOV2Q> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[giadefa]

The code is unit-neutral, whatever you use as input it spits out as output

…

On Mon, Oct 3, 2022 at 6:48 PM Peter Eastman ***@***.***> wrote: And another question: how do we want to handle units? Nothing in the current code worries about them, except sometimes for a particular dataset. The inputs and outputs to a model are just numbers that might have any units. But physical potentials involve hardcoded constants like eps_0 whose values depend on units. How should we handle this? Some possibilities include - Standardizing on a particular set of units for all models. - Standardize on a particular set of units for a particular potential function, and require the user to provide scaling factors for converting positions, energies, etc. to the required units. - Implement it at a higher level, where a model specifies what units it uses. - We could also make each Dataset specify its units, and conversions between the dataset and model units would happen automatically. — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3KUOSBZZSRRT5ZD6ARC5TWBNPCJANCNFSM47GPOV2Q> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[peastman]

The code is unit-neutral, whatever you use as input it spits out as output

I know. And that's incompatible with physics based potentials. They involve internal parameters with units. It's impossible to compute them without knowing what units the inputs are in, and what units the outputs are expected to be in.

[peastman]

can you use that single model as a wrapper for all the models that you would like to use? It's just the entry point.

Currently there's a single option in the configuration file, prior_model. It specifies the class of the single prior model to create, whose constructor has to take no arguments except the dataset. We need to be able to specify much more complicated arrangements. For example, "Include both a ZBL potential, using atomic numbers specified in the dataset, and a Coulomb potential, using charges that are generated as an output of the model."

[giadefa]

who builds the potential knows, so the problem is when we distribute it, maybe we can add some optional unit indication somehow?

…

On Tue, Oct 4, 2022 at 11:56 AM Peter Eastman ***@***.***> wrote: The code is unit-neutral, whatever you use as input it spits out as output I know. And that's incompatible with physics based potentials. They involve internal parameters with units. It's impossible to compute them without knowing what units the inputs are in, and what units the outputs are expected to be in. — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3KUOSNCH2E4YKKSGLTXC3WBRHUJANCNFSM47GPOV2Q> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[giadefa]

We could extend that as we did for the dataset arguments with a prior_args dictionary

…

On Tue, Oct 4, 2022 at 12:05 PM Peter Eastman ***@***.***> wrote: can you use that single model as a wrapper for all the models that you would like to use? It's just the entry point. Currently there's a single option in the configuration file, prior_model. It specifies the class of the single prior model to create, whose constructor has to take no arguments except the dataset. We need to be able to specify much more complicated arrangements. For example, "Include both a ZBL potential, using atomic numbers specified in the dataset, and a Coulomb potential, using charges that are generated as an output of the model." — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3KUORJGKTK5PHL2OVOPPLWBRIS7ANCNFSM47GPOV2Q> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

[peastman]

Perhaps we're thinking about this differently. Currently the file priors.py includes only a single prior model: Atomref, which subtracts a reference energy that is defined in the dataset for each atom type. My understanding is that we want to add more choices that implement common physical models. Someone should be able to specify prior_model: Coulomb in their configuration file, and it will add a Coulomb interaction to whatever model they're trying to train. Is that different from your understanding?

[giadefa]

yes, that is fine. Maybe I don't understand the problem, you can specify whatever class. Maybe just do what you need on a PR so we can discuss it. Especially if it's backward compatible, we can merge it. g

…

On Tue, Oct 4, 2022 at 12:15 PM Peter Eastman ***@***.***> wrote: Perhaps we're thinking about this differently. Currently the file priors.py includes only a single prior model: Atomref, which subtracts a reference energy that is defined in the dataset for each atom type. My understanding is that we want to add more choices that implement common physical models. Someone should be able to specify prior_model: Coulomb in their configuration file, and it will add a Coulomb interaction to whatever model they're trying to train. Is that different from your understanding? — Reply to this email directly, view it on GitHub <#26 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB3KUOWZ4NQG2VVUKFAVSTTWBRJZXANCNFSM47GPOV2Q> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>