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 simple save model test #1227

Merged
merged 6 commits into from
Jan 1, 2021
Merged

Add simple save model test #1227

merged 6 commits into from
Jan 1, 2021

Conversation

jeswan
Copy link
Collaborator

@jeswan jeswan commented Nov 5, 2020

This PR closes #1222 by adding save model tests for the simple runscript. These tests check the model saving behavior by:

  1. Overfitting a model and checking the last model weights are different than the best model weights
  2. Checking that only the best model is saved with the do_save_best argument.
  3. Checking that only the last model is saved with the do_save_last argument.

This PR also corrects a small bug in saving behavior (required for the tests to pass).

@codecov
Copy link

codecov bot commented Nov 12, 2020

Codecov Report

Merging #1227 (efacf8a) into master (1ab34a4) will decrease coverage by 0.02%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1227      +/-   ##
==========================================
- Coverage   55.88%   55.86%   -0.03%     
==========================================
  Files         149      149              
  Lines       10819    10825       +6     
==========================================
+ Hits         6046     6047       +1     
- Misses       4773     4778       +5     
Impacted Files Coverage Δ
jiant/utils/torch_utils.py 51.08% <16.66%> (-2.41%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ab34a4...efacf8a. Read the comment docs.

Copy link
Collaborator

@zphang zphang left a comment

Choose a reason for hiding this comment

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

Left a comment, but I'm fine merging for now.

tests/proj/simple/test_runscript.py Show resolved Hide resolved
@jeswan jeswan merged commit ce62495 into master Jan 1, 2021
@jeswan jeswan deleted the js/tests/save_model_simple_tests branch January 1, 2021 05:09
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.

Add additional tests for new metarunner saving behavior
2 participants