-
Notifications
You must be signed in to change notification settings - Fork 538
[Feature] Add Machine translation estimator in api #1156
base: v0.x
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1156 +/- ##
==========================================
+ Coverage 70.58% 73.77% +3.18%
==========================================
Files 72 76 +4
Lines 6970 7317 +347
==========================================
+ Hits 4920 5398 +478
+ Misses 2050 1919 -131
|
Job PR-1156/1 is complete. |
gnorm = gluon.utils.clip_global_norm(grads, self.clip) | ||
estimator.trainer.step(1) | ||
|
||
class TransformerGradientAccumulationHandler(GradientUpdateHandler, |
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.
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.
Why does grad acc handler API require batch_size
?
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.
It needs batch_size to rescale. See here https://github.com/dmlc/gluon-nlp/blob/master/scripts/machine_translation/train_transformer.py#L320
Job PR-1156/2 is complete. |
Job PR-1156/3 is complete. |
Job PR-1156/4 is complete. |
Job PR-1156/5 is complete. |
Job PR-1156/6 is complete. |
Job PR-1156/7 is complete. |
Job PR-1156/8 is complete. |
@eric-haibin-lin Do you think this pull request should be fixed on CI and then be merged? |
Description
Implementation of machine translation GNMT and transformer estimator.
Checklist
Essentials
Changes
Comments
cc @dmlc/gluon-nlp-team