-
Notifications
You must be signed in to change notification settings - Fork 322
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
Remove momentum updating from val step and add separate val queue #631
Remove momentum updating from val step and add separate val queue #631
Conversation
Codecov Report
@@ Coverage Diff @@
## master #631 +/- ##
==========================================
- Coverage 72.64% 72.25% -0.40%
==========================================
Files 119 119
Lines 7352 7356 +4
==========================================
- Hits 5341 5315 -26
- Misses 2011 2041 +30
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
@ananyahjha93 mind check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@maximzubkov Thank you for sending the fix! LGTM!
@ananyahjha93 Could you double-check the change here?
What does this PR do?
This pull request fixes the problem discussed in the following issues #615 #630
Before submitting
Changes:
_momentum_update_key_encoder
and_dequeue_and_enqueue
fromforward
val_queue
andval_queue_ptr
key
formforward
PR review