[API] Make BERT a hybrid block #877

eric-haibin-lin · 2019-08-15T00:43:43Z

Description

This PR makes use of the infer_range attribute of the arange operator (thanks to @TaoLv and @leezu ) in MXNet to fully hybridize the BERT model.

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

The change is backward compatible. Added tests for model forward with and without optional inputs.

…d-block

This reverts commit d024a0c.

…d-block

codecov · 2019-08-15T00:43:44Z

Codecov Report

❗ No coverage uploaded for pull request head (hybrid-block@7989539). Click here to learn what that means.
The diff coverage is n/a.

codecov · 2019-08-15T00:43:44Z

Codecov Report

Merging #877 into master will decrease coverage by 0.38%.
The diff coverage is 97.01%.

@@           Coverage Diff            @@
##           master   #877      +/-   ##
========================================
- Coverage   90.38%    90%   -0.39%     
========================================
  Files          66     66              
  Lines        6367   6433      +66     
========================================
+ Hits         5755   5790      +35     
- Misses        612    643      +31

Impacted Files	Coverage Δ
src/gluonnlp/model/bert.py	`99.45% <100%> (+0.06%)`	⬆️
src/gluonnlp/model/transformer.py	`89.69% <92.3%> (-0.44%)`	⬇️
src/gluonnlp/model/seq2seq_encoder_decoder.py	`45.31% <0%> (-29.69%)`	⬇️
src/gluonnlp/model/block.py	`51.92% <0%> (-1.93%)`	⬇️
src/gluonnlp/model/attention_cell.py	`93.28% <0%> (-1.35%)`	⬇️
src/gluonnlp/vocab/subwords.py	`81.29% <0%> (+0.16%)`	⬆️

mli · 2019-08-16T19:22:01Z

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

leezu

Why introduce legacy models? Can we just delete them?

eric-haibin-lin · 2019-08-17T23:09:38Z

@leezu the contrib.arange_like operator is not available in mxnet 1.5, which gluonnlp depends on

leezu · 2019-08-18T08:23:22Z

Right, but there seems to be the workaround based on infer_range which you are using in this PR. That works on mxnet 1.5. Where does the workaround fall short?

mli · 2019-08-18T22:11:40Z

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

mli · 2019-08-18T22:34:02Z

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

mli · 2019-08-18T22:53:07Z

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

mli · 2019-08-18T22:59:14Z

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

mli · 2019-08-18T23:12:41Z

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

mli · 2019-08-18T23:36:30Z

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

eric-haibin-lin · 2019-08-19T00:05:12Z

@leezu good point. I removed the legacy bert model definition. please review again

leezu

Thanks!

* change BaseTransformerEncoder forward * fix * merge _forward to hybrid_forward * if arange_like is not avaiable * fix * make transformer encoder a true hybridblock * fix lint * fix bug * use zeroslike and infer_range to avoid backprop problem * update test * Revert "update test" This reverts commit d024a0c. * more printing * fix test case * make bert hybrid * add legacy model * fix lint * fix lint * revert change for default parameter * fix arange dtype * fix lint * revert mokey patch in NMT interface * fix dtype * fix bug in arange * remove legacy model * also update bert scripts * fix lint * fix typo * remove hybridbert test

TaoLv and others added 21 commits July 10, 2019 17:13

change BaseTransformerEncoder forward

87565fd

Merge branch 'master' of https://github.com/dmlc/gluon-nlp into hybri…

fa66ec7

…d-block

fix

34d9f7d

Merge branch 'master' of https://github.com/dmlc/gluon-nlp into hybri…

23de820

…d-block

merge _forward to hybrid_forward

9f7af5e

if arange_like is not avaiable

702197e

fix

656f570

Merge remote-tracking branch 'upstream/master' into hybrid-block

d1501c8

make transformer encoder a true hybridblock

1001af2

fix lint

b4b2a24

fix bug

f613e56

use zeroslike and infer_range to avoid backprop problem

2d215ef

Merge remote-tracking branch 'upstream/master' into hybrid-block

8c03871

update test

d024a0c

Revert "update test"

a07538b

This reverts commit d024a0c.

Merge branch 'master' of https://github.com/dmlc/gluon-nlp into hybri…

fb2f4b5

…d-block

more printing

346a68e

fix test case

d0c1a2d

Merge remote-tracking branch 'upstream/master' into hybrid-block

1a76af7

make bert hybrid

4381076

fix conflict

7989539

eric-haibin-lin requested a review from szha as a code owner August 15, 2019 00:43

eric-haibin-lin and others added 5 commits August 14, 2019 22:41

add legacy model

cd1c9b0

fix lint

710d99d

fix lint

9ca4639

revert change for default parameter

741e2ce

fix arange dtype

267ee0c

eric-haibin-lin requested a review from TaoLv August 15, 2019 16:20

fix bug in arange

089bc3f

eric-haibin-lin requested review from szhengac, sxjscience, hhexiy, fierceX and leezu August 16, 2019 19:43

eric-haibin-lin mentioned this pull request Aug 17, 2019

[MODEL][API] Change BaseTransformerEncoder to HybridBlock #845

Closed

8 tasks

TaoLv approved these changes Aug 17, 2019

View reviewed changes

leezu reviewed Aug 17, 2019

View reviewed changes

eric-haibin-lin added 2 commits August 18, 2019 14:39

remove legacy model

01027f7

also update bert scripts

8190883

eric-haibin-lin added 2 commits August 18, 2019 15:17

fix lint

b3be351

fix typo

35ed771

remove hybridbert test

6f1f7d6

sxjscience approved these changes Aug 19, 2019

View reviewed changes

szhengac approved these changes Aug 19, 2019

View reviewed changes

leezu approved these changes Aug 19, 2019

View reviewed changes

leezu merged commit a9eb2fd into dmlc:master Aug 19, 2019

eric-haibin-lin deleted the hybrid-block branch February 2, 2020 06:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[API] Make BERT a hybrid block #877

[API] Make BERT a hybrid block #877

eric-haibin-lin commented Aug 15, 2019 •

edited

Loading

codecov bot commented Aug 15, 2019

codecov bot commented Aug 15, 2019 •

edited

Loading

mli commented Aug 16, 2019

leezu left a comment

eric-haibin-lin commented Aug 17, 2019

leezu commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

eric-haibin-lin commented Aug 19, 2019

leezu left a comment

[API] Make BERT a hybrid block #877

[API] Make BERT a hybrid block #877

Conversation

eric-haibin-lin commented Aug 15, 2019 • edited Loading

Description

Checklist

Essentials

Changes

Comments

codecov bot commented Aug 15, 2019

Codecov Report

codecov bot commented Aug 15, 2019 • edited Loading

Codecov Report

mli commented Aug 16, 2019

leezu left a comment

Choose a reason for hiding this comment

eric-haibin-lin commented Aug 17, 2019

leezu commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

mli commented Aug 18, 2019

eric-haibin-lin commented Aug 19, 2019

leezu left a comment

Choose a reason for hiding this comment

eric-haibin-lin commented Aug 15, 2019 •

edited

Loading

codecov bot commented Aug 15, 2019 •

edited

Loading