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

[API] Try to use the new None feature in MXNet + Drop support for MXNet 1.5 #967

Merged
merged 4 commits into from
Oct 17, 2019

Conversation

sxjscience
Copy link
Member

@sxjscience sxjscience commented Oct 10, 2019

Description

Since we have supported None in HybridBlock of Gluon (apache/mxnet#16280), remove all the hacks.

The change will force GluonNLP to rely on the nightly-build of MXNet.

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

Comments

  • It should be considered as a backward incompatible change because the user must upgrade to the latest MXNet

cc @dmlc/gluon-nlp-team

@sxjscience sxjscience requested a review from a team as a code owner October 10, 2019 16:59
@codecov
Copy link

codecov bot commented Oct 10, 2019

Codecov Report

Merging #967 into master will decrease coverage by 0.72%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #967      +/-   ##
==========================================
- Coverage   90.78%   90.05%   -0.73%     
==========================================
  Files          67       67              
  Lines        6423     6386      -37     
==========================================
- Hits         5831     5751      -80     
- Misses        592      635      +43
Impacted Files Coverage Δ
src/gluonnlp/model/transformer.py 91.07% <ø> (-0.14%) ⬇️
src/gluonnlp/model/language_model.py 98.49% <ø> (-0.05%) ⬇️
src/gluonnlp/model/train/cache.py 97.67% <ø> (-0.16%) ⬇️
src/gluonnlp/model/bert.py 91.58% <ø> (+1.32%) ⬆️
src/gluonnlp/model/train/language_model.py 96.21% <ø> (+0.87%) ⬆️
src/gluonnlp/model/sequence_sampler.py 74.56% <0%> (-17.08%) ⬇️
src/gluonnlp/model/attention_cell.py 94.4% <100%> (-0.16%) ⬇️

@leezu leezu mentioned this pull request Oct 10, 2019
@sxjscience sxjscience changed the title [OTHER][WIP] Try to use the new None feature in MXNet [OTHER][WIP] Try to use the new None feature in MXNet + Drop support for MXNet 1.5 Oct 10, 2019
@sxjscience sxjscience changed the title [OTHER][WIP] Try to use the new None feature in MXNet + Drop support for MXNet 1.5 [OTHER] Try to use the new None feature in MXNet + Drop support for MXNet 1.5 Oct 10, 2019
@sxjscience sxjscience changed the title [OTHER] Try to use the new None feature in MXNet + Drop support for MXNet 1.5 [API] Try to use the new None feature in MXNet + Drop support for MXNet 1.5 Oct 10, 2019
Copy link
Member

@eric-haibin-lin eric-haibin-lin left a comment

Choose a reason for hiding this comment

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

Are we going to replace mxnet nightly version with a custom pip wheel on s3?

sxjscience and others added 2 commits October 13, 2019 16:09
fix

fix

fix

remove all hack

try to remove all hack

remove print
Disable tests with MXNet 1.5 completely

Instead of running tests for nightly version twice

Try fix 'Inline literal start-string without end-string' error

Revert "Try fix 'Inline literal start-string without end-string' error"

This reverts commit b3b9e66.

Fix gpu_doc exit code

Improve the fix

Fix syntax

Escape $

Don't escape {

Revert "Don't escape {"

This reverts commit 976d455.

Revert "Escape $"

This reverts commit 9fd9e54.

Revert "Fix syntax"

This reverts commit 4ba73f6.

Revert "Improve the fix"

This reverts commit f62b817.

Revert "Fix gpu_doc exit code"

This reverts commit 0a3080e.
@mli
Copy link
Member

mli commented Oct 14, 2019

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

try to fix
@mli
Copy link
Member

mli commented Oct 14, 2019

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

@sxjscience
Copy link
Member Author

sxjscience commented Oct 14, 2019

Find that the SimVerb (test_simverb3500) is still not available, should we exclude it or upload it into S3?

@mli
Copy link
Member

mli commented Oct 14, 2019

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

@sxjscience
Copy link
Member Author

Are we going to replace mxnet nightly version with a custom pip wheel on s3?

We need to create one... I do not know how to do that though

@sxjscience sxjscience merged commit f512424 into dmlc:master Oct 17, 2019
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