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

[BUGFIX] Fix TokenEmbedding serialization with emb[emb.unknown_token] != 0#763

Merged
szha merged 2 commits intodmlc:masterfrom
leezu:tokembfixes
Jun 13, 2019
Merged

[BUGFIX] Fix TokenEmbedding serialization with emb[emb.unknown_token] != 0#763
szha merged 2 commits intodmlc:masterfrom
leezu:tokembfixes

Conversation

@leezu
Copy link
Contributor

@leezu leezu commented Jun 11, 2019

Follow-up on #750. #750 forgot to remove one leftover assertion, preventing serialization of TokenEmbedding with emb[emb.unknown_token] != 0.

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

@leezu leezu requested review from eric-haibin-lin and szha June 11, 2019 12:43
@codecov
Copy link

codecov bot commented Jun 11, 2019

Codecov Report

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

@codecov
Copy link

codecov bot commented Jun 11, 2019

Codecov Report

❗ No coverage uploaded for pull request base (master@96420aa). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #763   +/-   ##
=========================================
  Coverage          ?   90.59%           
=========================================
  Files             ?       64           
  Lines             ?     6112           
  Branches          ?        0           
=========================================
  Hits              ?     5537           
  Misses            ?      575           
  Partials          ?        0
Impacted Files Coverage Δ
src/gluonnlp/embedding/token_embedding.py 92.01% <ø> (ø)

@mli
Copy link
Member

mli commented Jun 11, 2019

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

@mli
Copy link
Member

mli commented Jun 11, 2019

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

@mli
Copy link
Member

mli commented Jun 12, 2019

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

@leezu leezu added bug Something isn't working release focus Progress focus for release labels Jun 12, 2019
@szha szha merged commit 2d3823e into dmlc:master Jun 13, 2019
@leezu leezu deleted the tokembfixes branch June 13, 2019 12:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working release focus Progress focus for release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants