Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement training on per atom energies by modifying loss modules #105

Merged
merged 11 commits into from
Feb 27, 2024
Merged

implement training on per atom energies by modifying loss modules #105

merged 11 commits into from
Feb 27, 2024

Conversation

SanggyuChong
Copy link
Contributor

@SanggyuChong SanggyuChong commented Feb 22, 2024

Changes implemented here were discussed with @frostedoyster and @Luthaf.

Model training on atomically averaged energies or other extensive targets can be achieved by specifying such peratom_targets, a list of strings, in the training section of the yaml files for the models. Note that this means future models that use the native training routine of metatensor-models should also specify this list in their default yaml files as well.

Then, prior to computing the loss, this list is checked, and for the targets that have been specified, both the model prediction and the reference target values are divided by the number of atoms. This takes place towards the end of the compute_model_loss function.

Resolves #95


📚 Documentation preview 📚: https://metatensor-models--105.org.readthedocs.build/en/105/

@SanggyuChong SanggyuChong marked this pull request as ready for review February 22, 2024 16:06
@frostedoyster
Copy link
Collaborator

The changes look good, but we need some documentation. In particular, the current implementation prints the RMSEs per atom of the quantities, while usually we print the RMSE per structure. We should say in the docs that the RMSE of per_atom quantities will also be printed per atom.

We should also check that all per_atom quantities are energies, at least for now

@frostedoyster frostedoyster merged commit 5726ef1 into metatensor:main Feb 27, 2024
8 checks passed
@Luthaf
Copy link
Member

Luthaf commented Feb 27, 2024

@frostedoyster please wait for CI to pass before merging =)

@SanggyuChong
Copy link
Contributor Author

Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option for per atom RMSE in the loss
4 participants