usnistgov / alignn

Atomistic Line Graph Neural Network https://scholar.google.com/citations?user=9Q-tNnwAAAAJ&hl=en https://www.youtube.com/watch?v=WYePjZMzx3M
https://jarvis.nist.gov/jalignn/
Other
236 stars 85 forks source link

Modified checkpoint handler to store best model #129

Closed pbenner closed 1 year ago

pbenner commented 1 year ago

Fixes #127

codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03% :tada:

Comparison is base (21e452c) 76.10% compared to head (20f9793) 76.14%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #129 +/- ## =========================================== + Coverage 76.10% 76.14% +0.03% =========================================== Files 23 23 Lines 3604 3609 +5 =========================================== + Hits 2743 2748 +5 Misses 861 861 ``` | [Files Changed](https://app.codecov.io/gh/usnistgov/alignn/pull/129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [alignn/train.py](https://app.codecov.io/gh/usnistgov/alignn/pull/129?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-YWxpZ25uL3RyYWluLnB5) | `86.12% <100.00%> (+0.13%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

knc6 commented 1 year ago

Hi @pbenner

Thank you for the PR. In order to keep the backward compatibility of the package, we should have two handlers: 1) using the trainer as well as 2) use your idea for saving on evaluator so that both last two steps and best evaluator models are saved. Are you open to adding this to your PR?

pbenner commented 1 year ago

Dear Kamal,

Sure, that's a good idea. I've just pushed the implementation to my branch.

Best, Philipp