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 global_step when gradient accumulation > 1 #832

Merged
merged 1 commit into from
Feb 16, 2020
Merged

Fix global_step when gradient accumulation > 1 #832

merged 1 commit into from
Feb 16, 2020

Conversation

peteriz
Copy link

@peteriz peteriz commented Feb 13, 2020

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos, doc improvements)
  • Did you read the contributor guideline?
  • Did you make sure to update the docs?
  • Did you write any new necessary tests?

What does this PR do?

Fix global_step update when gradient accumulation is > 1.
global_step will be updated only after all parts of the batch are done.
Fixes #831

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

@peteriz peteriz requested a review from Borda February 13, 2020 11:02
@@ -426,7 +426,9 @@ def run_training_epoch(self):
# logs user requested information to logger
self.log_metrics(batch_step_metrics, grad_norm_dic)

self.global_step += 1
# progress global step according to grads progress
if (self.batch_idx + 1) % self.accumulate_grad_batches == 0:
Copy link
Member

@Borda Borda Feb 14, 2020

Choose a reason for hiding this comment

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

can we clarify what is the meaning of these two variables?
could you pls add comment to the trainer init starting with #: so it is generated also in the documentation...
Thx

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.

global_step advanced between accumulations if gradient_accumulation > 1
3 participants