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

[BUGFIX] Update default alpha for sentencepiece tokenizer#1247

Closed
haven-jeon wants to merge 1 commit intodmlc:v0.9.xfrom
haven-jeon:g_v0.9.x
Closed

[BUGFIX] Update default alpha for sentencepiece tokenizer#1247
haven-jeon wants to merge 1 commit intodmlc:v0.9.xfrom
haven-jeon:g_v0.9.x

Conversation

@haven-jeon
Copy link
Copy Markdown
Member

Description

#1239

Checklist

Essentials

  • [v] PR's title starts with a category (e.g. [BUGFIX], [MODEL], [TUTORIAL], [FEATURE], [DOC], etc)
  • [v] Changes are complete (i.e. I finished coding on this PR)
  • [v] All changes have test coverage
  • [v] 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

@haven-jeon haven-jeon requested a review from szha June 11, 2020 04:49
@haven-jeon haven-jeon requested a review from a team as a code owner June 11, 2020 04:49
@sxjscience
Copy link
Copy Markdown
Member

Not sure why the CI is failing. But the change looks good.

@chenw23
Copy link
Copy Markdown
Member

chenw23 commented Jun 11, 2020

Please don't worry. I will look into the failure reason soon.

@chenw23
Copy link
Copy Markdown
Member

chenw23 commented Jun 11, 2020

Hello @haven-jeon
This failure is due to an error that is already fixed in the master branch #1236
For the gluon-nlp repo, master branch is its dev branch and v0.9.x is its release branch. Is there urgent need to change the code directly to the release branch rather on the dev branch?
If it is possible to commit to master branch, would you please rebase your pull request on top of the current master branch?
The failure will vanish automatically if you are using code on the master branch.
Thanks!

@haven-jeon
Copy link
Copy Markdown
Member Author

Hello @haven-jeon
This failure is due to an error that is already fixed in the master branch #1236
For the gluon-nlp repo, master branch is its dev branch and v0.9.x is its release branch. Is there urgent need to change the code directly to the release branch rather on the dev branch?
If it is possible to commit to master branch, would you please rebase your pull request on top of the current master branch?
The failure will vanish automatically if you are using code on the master branch.
Thanks!

If it is an already PR on master, it would be better to cancel the PR. :)

@haven-jeon haven-jeon closed this Jun 15, 2020
@chenw23
Copy link
Copy Markdown
Member

chenw23 commented Jun 15, 2020

Sorry, Maybe I didn't express clearly about it.
I am saying that the bug leading to the CI failure is fixed on the master branch. I am not saying the problem you are trying to address is fixed on the master branch.
You can feel free to give this pull request on the master branch, where the CI is working well.

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.

3 participants