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.

Sign up for GitHub

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

Jump to bottom

Add gradient calculators #26

Merged
merged 10 commits into from
Jan 13, 2024
Merged

Add gradient calculators #26

merged 10 commits into from
Jan 13, 2024

Conversation

frostedoyster
Copy link
Collaborator

@frostedoyster frostedoyster commented Jan 4, 2024 •
edited
Loading

This is a PR that works mainly on gradients.

  • Gradients calculators have been added with respect to positions (for forces) and displacements (needed for virials/stresses)
  • A temporary loss function that can handle multiple outputs as well as gradients (temporary, undocumented, to be moved to metatensor-learn)
  • The model now supports multiple outputs (but the trainer doesn't yet)
  • Spotted an issue in the regression tests: the results change based on whether the test is run on its own or together with the other tests in its folder (@PicoCentauri this could explain the problems we saw a while ago)
  • Allowing training on GPU should now be trivial, but it is left for a future PR

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

@frostedoyster frostedoyster marked this pull request as ready for review January 11, 2024 23:28
@frostedoyster frostedoyster changed the title Add gradient calculator Add gradient calculators Jan 11, 2024
PicoCentauri
PicoCentauri requested changes Jan 12, 2024
View reviewed changes
Copy link
Contributor

@PicoCentauri PicoCentauri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good addition with the gradients! I have some ideas that may improve the logic.

src/metatensor/models/soap_bpnn/model.py Outdated Show resolved Hide resolved
src/metatensor/models/utils/loss.py Outdated Show resolved Hide resolved
src/metatensor/models/utils/loss.py Show resolved Hide resolved
src/metatensor/models/utils/loss.py Show resolved Hide resolved
src/metatensor/models/utils/loss.py Show resolved Hide resolved
src/metatensor/models/utils/output_gradient.py Show resolved Hide resolved
src/metatensor/models/utils/compute_loss.py Show resolved Hide resolved
PicoCentauri
PicoCentauri approved these changes Jan 13, 2024
View reviewed changes
Copy link
Contributor

@PicoCentauri PicoCentauri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes. This is good to go now :-)

@frostedoyster frostedoyster merged commit 51df872 into main Jan 13, 2024
7 checks passed
@frostedoyster frostedoyster deleted the forces-virials branch January 13, 2024 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PicoCentauri PicoCentauri PicoCentauri approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@frostedoyster @PicoCentauri