Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mxnet 1.5.0 compatibility #245

Closed
wants to merge 5 commits into from
Closed

Conversation

lostella
Copy link
Contributor

Description of changes: MXNet requirement was set to <1.5.* in #215 due to some incompatibility with mxnet-1.5.0. By running tests, it seems like the only problem was in the code for WaveNet. This PR updates the problematic lines to make our WaveNet implementation compatible with mxnet-1.5.0.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@lostella lostella requested a review from vafl August 14, 2019 12:05
@lostella
Copy link
Contributor Author

Errata: it looks like there's more issues than the one I addressed in the initial commit (despite the fact all tests are passing locally :-/). Diving deeper.

@lostella
Copy link
Contributor Author

I'm not sure what's running in the tests: these logs mention at some point

# packages in environment at /var/lib/jenkins/workspace/workspace/gluon-ts-cpu-py3/conda/cpu/py3-master:
#
# Name                    Version                   Build  Channel
[...]
mxnet-mkl                 1.4.1                     <pip>
[...]
numpy                     1.14.6                    <pip>

But if I locally install it the same way as in CI (using pip install -v -e ".[shell,Prophet]", see here) then I get mxnet-1.5.0 and numpy-1.17.0.

Could it be some caching messing up with this? @szha

@lostella
Copy link
Contributor Author

Nevermind: I think this problem is caused by #131

@lostella lostella force-pushed the mxnet-1.5.0 branch 2 times, most recently from 316029f to 9625eef Compare August 27, 2019 15:16
@lostella
Copy link
Contributor Author

@szha
Copy link
Member

szha commented Sep 11, 2019

@reminisce

@lostella
Copy link
Contributor Author

Related mxnet issue: apache/mxnet#16135

@strawberrypie
Copy link
Contributor

The fix for the issue mentioned above is merged into mxnet: apache/mxnet#16139
Sadly, it was done after the release of 1.5.1: https://github.com/apache/incubator-mxnet/releases/tag/1.5.1
What is the plan moving forward regarding mxnet version? Updating to mxnet 1.5 will allow the project to remove Trainer class and replace it with gluon.contrib.Estimator — which supports things like early stopping.

@lostella
Copy link
Contributor Author

lostella commented Nov 5, 2019

@strawberrypie good point, that’s something which is worth looking into. Although I think that’s kind of an experimental API of MXNet’s? Another thing that we’ll look into is the numpy-compatible arrays API, see apache/mxnet#14253

@lostella
Copy link
Contributor Author

lostella commented Nov 5, 2019

Btw I think this PR can be closed in favor of a new one whenever the mxnet version can be bumped

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants