torchmd-net icon indicating copy to clipboard operation
torchmd-net copied to clipboard

Inconsistency between gradients and forces

Open raimis opened this issue 1 year ago • 9 comments

When training with forces, the model computes the negative gradient of the energy with respect to the positions (a.k.a. forces): https://github.com/torchmd/torchmd-net/blob/db72e12461b1bce9bb3ddebd093fa11a803d37ca/torchmdnet/models/model.py#L196-L207

Some the loaders load forces: https://github.com/torchmd/torchmd-net/blob/db72e12461b1bce9bb3ddebd093fa11a803d37ca/torchmdnet/datasets/ani.py#L279-L281 https://github.com/torchmd/torchmd-net/blob/db72e12461b1bce9bb3ddebd093fa11a803d37ca/torchmdnet/datasets/comp6.py#L100-L102

While the other ones load gradients (the opposite sign): https://github.com/torchmd/torchmd-net/blob/db72e12461b1bce9bb3ddebd093fa11a803d37ca/torchmdnet/datasets/qm9q.py#L147-L151 https://github.com/torchmd/torchmd-net/blob/db72e12461b1bce9bb3ddebd093fa11a803d37ca/torchmdnet/datasets/spice.py#L99-L103

We need to agree what dy represents.

raimis avatar Aug 25 '22 12:08 raimis

IMO dy should be the derivative of y i.e. the energy. Meaning the negative of the force. We should not store forces in dy

stefdoerr avatar Aug 25 '22 12:08 stefdoerr

Ping: @PhilippThoelke @giadefa

raimis avatar Aug 25 '22 12:08 raimis

Also checking the loss function code and comments, it seems dy is interpreted as gradients (not forces).

https://github.com/torchmd/torchmd-net/blob/db72e12461b1bce9bb3ddebd093fa11a803d37ca/torchmdnet/module.py#L71-L132

raimis avatar Aug 26 '22 08:08 raimis

The model outputs force predictions. https://github.com/torchmd/torchmd-net/blob/main/torchmdnet/models/model.py#L207

I agree that the naming is inconsistent but as far as I remember, when I implemented QM9, MD17 and ANI-1, they all loaded forces instead of the derivative. The model was consistent with that.

I'm not sure about the more recently added dataset loaders.

PhilippThoelke avatar Aug 26 '22 14:08 PhilippThoelke

The HDF5 loader returns forces, because that's what the model expects. I think it took me a while to figure that out, and I was certainly surprised when I realized dy actually meant the negative gradient! Renaming y and dy to energy and forces would make it a lot clearer.

peastman avatar Aug 26 '22 15:08 peastman

@peastman is SPICE returning forces or gradients?

        all_dy = (
            pt.tensor(mol["dft_total_gradient"], dtype=pt.float32)
            * self.HARTREE_TO_EV
            / self.BORH_TO_ANGSTROM
        )

giadefa avatar Aug 26 '22 15:08 giadefa

I'm not sure. Raimondas wrote that class.

peastman avatar Aug 26 '22 15:08 peastman

so you are not using that loader? Which one are you using?

On Fri, Aug 26, 2022 at 5:52 PM Peter Eastman @.***> wrote:

I'm not sure. Raimondas wrote that class.

— Reply to this email directly, view it on GitHub https://github.com/torchmd/torchmd-net/issues/116#issuecomment-1228663228, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3KUORH7ALKWWRKIMPOIOTV3DR2ZANCNFSM57S7D4XA . You are receiving this because you were mentioned.Message ID: @.***>

giadefa avatar Aug 26 '22 15:08 giadefa

HDF5.

peastman avatar Aug 26 '22 15:08 peastman

The datasets measure forces and I would expect that they store forces

On Thu, Aug 25, 2022 at 2:51 PM Raimondas Galvelis @.***> wrote:

Ping: @PhilippThoelke https://github.com/PhilippThoelke @giadefa https://github.com/giadefa

— Reply to this email directly, view it on GitHub https://github.com/torchmd/torchmd-net/issues/116#issuecomment-1227216646, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3KUOR2S7BHHATFIHDKKWDV25T63ANCNFSM57S7D4XA . You are receiving this because you were mentioned.Message ID: @.***>

giadefa avatar Oct 11 '22 08:10 giadefa