microsoft / Graphormer

Graphormer is a general-purpose deep learning backbone for molecular modeling.
MIT License
2k stars 324 forks source link

Puzzled about the calculation of energy MAE loss #142

Closed whu-dft closed 1 year ago

whu-dft commented 1 year ago

Thanks for your great work! Recently, I began to read the codes but encountered a problem. In the IS2RE task, I found the way to calculate the energy loss is in the following codes link.

        relaxed_energy = sample["targets"]["relaxed_energy"]
        relaxed_energy = relaxed_energy.float()
        relaxed_energy = (relaxed_energy - self.e_mean) / self.e_std
        sample_size = relaxed_energy.numel()
        loss = F.l1_loss(output.float().view(-1), relaxed_energy, reduction="none")
        ...
        loss = loss.sum()

Then when print the loss, you do the following conversion link:

        loss_sum = sum(log.get("loss", 0) for log in logging_outputs)
        ...
        sample_size = sum(log.get("sample_size", 0) for log in logging_outputs)
        mean_loss = (loss_sum / sample_size) * IS2RECriterion.e_std

After careful consideration,I can't agree that the above result is equal to the original meaning of MAE of energy, which should simply be :

loss_sum = F.l1_loss(output, relaxed_energy, reduction="none").sum()
loss = loss_sum / sample_size

Since this problem should be important, hope to receive your kindly response.

Best regards.

zhengsx commented 1 year ago

Would you elaborate more about your question?

whu-dft commented 1 year ago

Hi, thanks for your attention. I read a few papers these days, and found that the metric that Graphormer used is the std. MAE. The std. MAE originates from the paper, and is deployed to metric the errors of many different targets, such as energy, HOMO, LUMO in the QM9 datasets.

image

And the expression is like:

image

However, for the IS2RE task in Graphormer the only metric is the energy $U_0$. I think the std. MAE should not be applied to IS2RE task. In another word, whether the way to metric the relaxation energy in Graphormer is inappropriate? I think we only need to calculate the MAE as the original definition $\left|E-E0\right|$.

Would you elaborate more about your question?

whu-dft commented 1 year ago

@zhengsx Hi, I feel sorry to tell you that I have made a mistake. The way Graphormer calculates energy MAE loss is quit right. It is me who misunderstanded the method to handle it.