Skip to content

Conversation

@khazit
Copy link
Contributor

@khazit khazit commented Apr 27, 2021

What does this PR do?

When using ModelCheckpoint in verbose mode, there are two different logs that you can get :

  • If your model did not improve, you get Epoch 1, step 100: val_loss was not in top 1
  • If your model did improve, you get Epoch 1, global step 100: val_loss was not in top 1

In the first case, the global keyword is missing, and it can be quite confusing.
I was stuck for some time as I was trying to figure out why I had the following output:

Epoch 1, step 48: val_loss was not in top 1
Epoch 2, step 96: val_loss was not in top 1
Epoch 3, step 144: val_loss was not in top 1

I believed that each epoch the callback was looking at a specific step for the validation loss.

Adding the word global makes it consistent with the message displayed if the model has improved.

I did not open an issue, as it's a simple fix.

Did you have fun?

Make sure you had fun coding 🙃

@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #7244 (351c11f) into master (94fcaaf) will decrease coverage by 4%.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master   #7244    +/-   ##
=======================================
- Coverage      91%     87%    -4%     
=======================================
  Files         199     199            
  Lines       12783   12783            
=======================================
- Hits        11662   11150   -512     
- Misses       1121    1633   +512     

@carmocca carmocca added ready PRs ready to be merged docs Documentation related callback labels Apr 27, 2021
@carmocca carmocca added this to the v1.3 milestone Apr 27, 2021
@rohitgr7 rohitgr7 enabled auto-merge (squash) April 28, 2021 06:25
@rohitgr7 rohitgr7 merged commit cbc6e30 into Lightning-AI:master Apr 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

callback docs Documentation related ready PRs ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants