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

fix Display wrong epoch on Keras resume training #1150

Merged
merged 2 commits into from May 3, 2021

Conversation

javoweb
Copy link
Contributor

@javoweb javoweb commented Mar 19, 2021

This fix TqdmCallback showing wrong information about epochs when resume training with initial_epoch != 0 as was reported on #1138

The test has been provided but also the Colab example provided by @fabiocarrara here:

@javoweb javoweb requested a review from casperdcl as a code owner March 19, 2021 16:38
@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #1150 (74ec622) into devel (d9372a2) will decrease coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            devel    #1150      +/-   ##
==========================================
- Coverage   89.80%   89.76%   -0.05%     
==========================================
  Files          26       26              
  Lines        1707     1710       +3     
  Branches      283      284       +1     
==========================================
+ Hits         1533     1535       +2     
- Misses        128      129       +1     
  Partials       46       46              

tqdm/keras.py Outdated
Comment on lines 76 to 77
if self.epoch_bar.n < epoch:
self.epoch_bar.update(epoch-self.epoch_bar.n)
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Is this only ever triggered exactly once when training is resumed? Or are there any other circumstances?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly once. I don't figure out other circumstances.

@@ -69,10 +69,12 @@ def __init__(self, epochs=None, data_size=None, batch_size=None, verbose=1,
def on_train_begin(self, *_, **__):
params = self.params.get
auto_total = params('epochs', params('nb_epoch', None))
if auto_total is not None:
if auto_total is not None and auto_total != self.epoch_bar.total:
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid re-assigning the total when it is provided manually by the user.

@casperdcl casperdcl added p3-enhancement 🔥 Much new such feature submodule ⊂ Periphery/subclasses to-fix ⌛ In progress labels Mar 19, 2021
@casperdcl casperdcl added this to In Progress in Casper Mar 19, 2021
@casperdcl casperdcl added this to the Non-breaking milestone Mar 19, 2021
@casperdcl casperdcl changed the base branch from master to devel May 2, 2021 23:26
- set initial epochs
- update tests
@casperdcl casperdcl added c1-quick 🕐 Complexity low p2-bug-warning ⚠ Visual output bad to-merge ↰ Imminent and removed to-fix ⌛ In progress labels May 3, 2021
@casperdcl casperdcl moved this from In Progress to Next Release in Casper May 3, 2021
@casperdcl casperdcl merged commit 9aea3ae into tqdm:devel May 3, 2021
Casper automation moved this from Next Release to Done May 3, 2021
@casperdcl casperdcl mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c1-quick 🕐 Complexity low p2-bug-warning ⚠ Visual output bad p3-enhancement 🔥 Much new such feature submodule ⊂ Periphery/subclasses to-merge ↰ Imminent
Projects
Casper
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants