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

Save best model #365

Merged
merged 9 commits into from
Aug 12, 2024
Merged

Save best model #365

merged 9 commits into from
Aug 12, 2024

Conversation

Lilferrit
Copy link
Contributor

Save model with the lowest validation loss to the checkpoint file "best.ckpt". This is implemented by locking save_top_k on the validation loss callback to 1 and eliminating the save_top_k option from the Casanovo config.

Copy link

codecov bot commented Aug 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.06%. Comparing base (9e3b630) to head (d23583d).
Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #365      +/-   ##
==========================================
+ Coverage   91.48%   94.06%   +2.57%     
==========================================
  Files          12       12              
  Lines        1022     1027       +5     
==========================================
+ Hits          935      966      +31     
+ Misses         87       61      -26     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@bittremieux bittremieux left a comment

Choose a reason for hiding this comment

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

Looks good. One thing to add is to give a warning to the user that the save_top_k setting has been removed. The current deprecation warning can be modified to report removal in addition to renaming of settings (e.g. by setting the value to None and modifying the warning a bit further down).

Additionally, this should be mentioned in the changelog.

@Lilferrit Lilferrit requested a review from bittremieux August 7, 2024 21:52
casanovo/config.py Outdated Show resolved Hide resolved
casanovo/config.py Show resolved Hide resolved
casanovo/config.py Outdated Show resolved Hide resolved
@Lilferrit Lilferrit requested a review from bittremieux August 8, 2024 22:55
casanovo/config.py Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
tests/unit_tests/test_config.py Outdated Show resolved Hide resolved
@Lilferrit Lilferrit merged commit ba58668 into dev Aug 12, 2024
6 checks passed
@Lilferrit Lilferrit deleted the save-best branch August 12, 2024 17:10
@Lilferrit Lilferrit linked an issue Aug 12, 2024 that may be closed by this pull request
Lilferrit added a commit that referenced this pull request Aug 21, 2024
* prediction output in model eval mode

* eliminate eval command, introduce -e flag for predict command

* adapted unit test to new model runner and model functionality

* updated documentation

* removed log and result files

* Generate new screengrabs with rich-codex

* Update paper reference (#361)

* Bug report template (#360)

* bug report template

* punctuation, hardware description item

* Restrict NumPy to pre-2.0 (#344)

* Restrict NumPy to pre-2.0

* Update changelog

* Update paper reference (#361)

---------

Co-authored-by: Lilferrit <[email protected]>

* upgrade codecove to v4 (#364)

* implemen eval mode at model runner level, fix unit test

* CLI documentation

* Generate new screengrabs with rich-codex

* requested changes

* Generate new screengrabs with rich-codex

* evaluation test cases

* file warnings, evaluation tests

* fixed ubuntu specific test case bug

* verify annotated mgf files

* verify annotated mgf files

* Generate new screengrabs with rich-codex

* Save best model (#365)

* save best model

* save best model

* updated unit tests

* remove save top k config item

* added save_top_k to deprecated config options

* changelog entry

* test case, formatting

* requested changes

* prediction output in model eval mode

* eliminate eval command, introduce -e flag for predict command

* adapted unit test to new model runner and model functionality

* updated documentation

* removed log and result files

* implemen eval mode at model runner level, fix unit test

* CLI documentation

* Bug report template (#360)

* bug report template

* punctuation, hardware description item

* Restrict NumPy to pre-2.0 (#344)

* Restrict NumPy to pre-2.0

* Update changelog

* Update paper reference (#361)

---------

Co-authored-by: Lilferrit <[email protected]>

* requested changes

* evaluation test cases

* file warnings, evaluation tests

* fixed ubuntu specific test case bug

* verify annotated mgf files

* AnnotatedSpectrumIndex type error

* requested changes, changelog entry

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Wout Bittremieux <[email protected]>
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.

Designate a filename for the "best" model
2 participants