Skip to content
This repository has been archived by the owner on Jan 15, 2024. It is now read-only.

[Feature] Add Machine translation estimator in api #1156

Open
wants to merge 26 commits into
base: v0.x
Choose a base branch
from

Conversation

liuzh47
Copy link
Contributor

@liuzh47 liuzh47 commented Feb 13, 2020

Description

Implementation of machine translation GNMT and transformer estimator.

Checklist

Essentials

  • PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage
  • Code is well-documented

Changes

  • Feature1, tests, (and when applicable, API doc)
  • Feature2, tests, (and when applicable, API doc)

Comments

  • If this change is a backward incompatible change, why must this change be made.
  • Interesting edge cases to note here

cc @dmlc/gluon-nlp-team

@liuzh47 liuzh47 requested a review from a team as a code owner February 13, 2020 13:16
@codecov
Copy link

codecov bot commented Feb 13, 2020

Codecov Report

Merging #1156 into master will increase coverage by 3.18%.
The diff coverage is 26.22%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
src/gluonnlp/estimator/__init__.py 100% <100%> (ø)
...nlp/estimator/machine_translation_event_handler.py 22.22% <22.22%> (ø)
...p/estimator/machine_translation_batch_processor.py 28.57% <28.57%> (ø)
...luonnlp/estimator/machine_translation_estimator.py 54.54% <54.54%> (ø)
src/gluonnlp/data/translation.py 26.35% <0%> (-73.65%) ⬇️
src/gluonnlp/model/train/cache.py 25.58% <0%> (-72.1%) ⬇️
src/gluonnlp/model/transformer.py 31.73% <0%> (-54.81%) ⬇️
src/gluonnlp/data/batchify/language_model.py 43.92% <0%> (-52.34%) ⬇️
src/gluonnlp/model/translation.py 20.31% <0%> (-51.57%) ⬇️
src/gluonnlp/embedding/evaluation.py 40.33% <0%> (-51.27%) ⬇️
... and 45 more

@mli
Copy link
Member

mli commented Feb 13, 2020

Job PR-1156/1 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/1/index.html

gnorm = gluon.utils.clip_global_norm(grads, self.clip)
estimator.trainer.step(1)

class TransformerGradientAccumulationHandler(GradientUpdateHandler,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mli
Copy link
Member

mli commented Feb 14, 2020

Job PR-1156/2 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/2/index.html

@mli
Copy link
Member

mli commented Feb 14, 2020

Job PR-1156/3 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/3/index.html

@mli
Copy link
Member

mli commented Feb 14, 2020

Job PR-1156/4 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/4/index.html

@mli
Copy link
Member

mli commented Feb 14, 2020

Job PR-1156/5 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/5/index.html

@mli
Copy link
Member

mli commented Feb 14, 2020

Job PR-1156/6 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/6/index.html

@mli
Copy link
Member

mli commented Feb 17, 2020

Job PR-1156/7 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/7/index.html

@mli
Copy link
Member

mli commented Feb 17, 2020

Job PR-1156/8 is complete.
Docs are uploaded to http://gluon-nlp-staging.s3-accelerate.dualstack.amazonaws.com/PR-1156/8/index.html

@chenw23
Copy link
Member

chenw23 commented Jun 19, 2020

@eric-haibin-lin Do you think this pull request should be fixed on CI and then be merged?

@szha szha changed the base branch from master to v0.x August 13, 2020 02:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants