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

[BugFix] support int32 for sampled blocks #1106

Merged
merged 2 commits into from
Jan 14, 2020
Merged

[BugFix] support int32 for sampled blocks #1106

merged 2 commits into from
Jan 14, 2020

Conversation

eric-haibin-lin
Copy link
Member

Description

Support int32 candidate ids and labels for NCE/IS blocks.

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

@eric-haibin-lin eric-haibin-lin requested a review from a team as a code owner January 14, 2020 01:37
@codecov
Copy link

codecov bot commented Jan 14, 2020 •
edited
Loading

Codecov Report

Merging #1106 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1106      +/-   ##
==========================================
+ Coverage   88.25%   88.25%   +<.01%     
==========================================
  Files          67       67              
  Lines        6274     6275       +1     
==========================================
+ Hits         5537     5538       +1     
  Misses        737      737
Impacted Files Coverage Δ
src/gluonnlp/model/sampled_block.py 90.74% <100%> (+0.08%) ⬆️

@mli
Copy link
Member

mli commented Jan 14, 2020

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

@mli
Copy link
Member

mli commented Jan 14, 2020

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

leezu
leezu approved these changes Jan 14, 2020
View reviewed changes
Copy link
Contributor

@leezu leezu left a comment

Choose a reason for hiding this comment

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

LGTM. Should int64 support be added at some point in time?

@eric-haibin-lin
Copy link
Member Author

Maybe in the future. int32 supports up to 2 billion IDs and should be sufficient for most use cases.

@eric-haibin-lin eric-haibin-lin merged commit f8a4318 into dmlc:master Jan 14, 2020
eric-haibin-lin added a commit to eric-haibin-lin/gluon-nlp that referenced this pull request Jan 14, 2020
@eric-haibin-lin
[BugFix] support int32 for sampled blocks (dmlc#1106)
26ec61a 
* support int32 for sampled blocks

* Fix lint
@eric-haibin-lin eric-haibin-lin mentioned this pull request Jan 14, 2020
Merged
6 tasks
eric-haibin-lin added a commit that referenced this pull request Jan 14, 2020
@eric-haibin-lin
[bugfix] Backport int32 fix for sampled block and bump version (#1109)
75b3c12 
* [BugFix] support int32 for sampled blocks (#1106)

* support int32 for sampled blocks

* Fix lint

* Update version
@eric-haibin-lin eric-haibin-lin deleted the int32-sample branch February 2, 2020 06:21
@chenw23 chenw23 mentioned this pull request Feb 11, 2020
Merged
6 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@leezu leezu leezu approved these changes

@szha szha Awaiting requested review from szha

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eric-haibin-lin @mli @leezu