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

Add evaluation timings #353

Merged
merged 2 commits into from
Oct 9, 2024
Merged

Add evaluation timings #353

merged 2 commits into from
Oct 9, 2024

Conversation

frostedoyster
Copy link
Collaborator

@frostedoyster frostedoyster commented Oct 4, 2024

Adds timings (per atom) to the evaluation script, as in #338

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

📚 Documentation preview 📚: https://metatrain--353.org.readthedocs.build/en/353/

@frostedoyster
Copy link
Collaborator Author

@spozdn as well (can't request your review for now)

@abmazitov
Copy link
Contributor

Thank you @frostedoyster , this is indeed very useful!!

I have two questions: shall we also add the std value of timing to check if the performance is uniform? And do we have the evaluation done with the batch size of one, or we can actually set the batch size?

@frostedoyster
Copy link
Collaborator Author

frostedoyster commented Oct 4, 2024

I like adding the std (although one should keep in mind that it might be high if the sizes of the structures aren't uniform). We're still using a batch size of one, and this is still hardcoded (the user can't change it).

Adding a batch size for eval would be a bit challenging for our API because eval, for now, takes no options. This means that we should have options for eval and, since eval is called after train, an eval section in the training options. I think that should be part of a different PR. I think this PR is still useful however because one structure at a time is pretty much what you would get if you did MD on the test set.

@frostedoyster frostedoyster merged commit 8e60e12 into main Oct 9, 2024
12 checks passed
@frostedoyster frostedoyster deleted the eval-timings branch October 9, 2024 07:51
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.

2 participants