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

Add dockerfiles for the conda package builds #3344

Merged
merged 19 commits into from
Jul 2, 2019

Conversation

abergeron
Copy link
Contributor

I'm not 100% sure what the procedure to add new docker images is. I figure I need to get the images added to docker hub under tvmai and then use the tags for those images in the Jenkinsfile.

I've tested the commands to ensure that building the docker images and running the builds in them works, but I can't upload the built images to the tvmai docker hub myself.

@tqchen

@abergeron
Copy link
Contributor Author

Don't merge this, I discovered some problems in the resulting packages. I'll fix those problems and add some automated testing of the resulting packages.

@abergeron abergeron force-pushed the cuda_docker branch 2 times, most recently from f43cc4a to 00f7c66 Compare June 13, 2019 21:01
@abergeron
Copy link
Contributor Author

Ok now it seems to be good. There are some major changes that I'll explain after. The main point is that this introduces new Dockerfiles for the build and those have to be cached. I think I can't do that part myself.

Once that is done, I will update the Jenkinsfile so that the packages are actually built as part of the tests so we know they are good and don't keep breaking :)

As for the changes, the tvm, topi and nnvm python packages are now a single package. This is because the there are some parts of tvm that depend on topi. It doesn't change the way the source is organized, only that there is a single conda package for all of them.

I've bumped the conda version to 0.6.0dev1 since I want to make sure that there will not be old topi/nnvm packages left behind to conflict with the new ones. This will fix itself when 0.7.0 comes around.

This also enables support for Metal in the OS X builds.

@@ -0,0 +1,5 @@
#!/bin/sh
conda build tvm-libs
Copy link
Contributor

Choose a reason for hiding this comment

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

For robustness it is good practice to include at least:
set -e
set -u
at the top of every .sh script. This will ensure that if any command in the script fails, the error is propagated rather than elided away, it will also ensure that scripts using undefined variables fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll do that.

@abergeron
Copy link
Contributor Author

Can I get a review of this? It is done now. @tqchen

@tqchen
Copy link
Member

tqchen commented Jun 27, 2019

@tqchen tqchen merged commit 988ea2a into apache:master Jul 2, 2019
@tqchen
Copy link
Member

tqchen commented Jul 2, 2019

THanks, @abergeron , this PR is now merged

wweic pushed a commit to wweic/tvm that referenced this pull request Jul 11, 2019
* First shot

* Add dockerfile for CPU too

* Finish the build infrastructure

* Remove extra file

* Comment out the Jenkinsfile section since it is not ready

* Add missing license headers

* Update to newer cudnn that anaconda packaged

* Bump the build numbers for the newer cudnn

* Bring back the toolchain option with a tweak for cuda

* Cache some large packages in the docker and update to llvm 7.0.0

* Merge all the python packages together

* First fix for the conda cuda builds (again)

* Use the tarball version of cudnn since tvm has trouble detecting the other one

* Use llvm 8.0 from the numba packages

* Also use llvm 8.0 for the cpu builds

* Don't use the anaconda compiler for OS X

* Enable Metal on OS X builds

* Make sure to detect undefined variables in scripts

* Fix build when not using cuda
wweic pushed a commit to wweic/tvm that referenced this pull request Jul 11, 2019
* First shot

* Add dockerfile for CPU too

* Finish the build infrastructure

* Remove extra file

* Comment out the Jenkinsfile section since it is not ready

* Add missing license headers

* Update to newer cudnn that anaconda packaged

* Bump the build numbers for the newer cudnn

* Bring back the toolchain option with a tweak for cuda

* Cache some large packages in the docker and update to llvm 7.0.0

* Merge all the python packages together

* First fix for the conda cuda builds (again)

* Use the tarball version of cudnn since tvm has trouble detecting the other one

* Use llvm 8.0 from the numba packages

* Also use llvm 8.0 for the cpu builds

* Don't use the anaconda compiler for OS X

* Enable Metal on OS X builds

* Make sure to detect undefined variables in scripts

* Fix build when not using cuda
wweic pushed a commit to neo-ai/tvm that referenced this pull request Jul 11, 2019
* First shot

* Add dockerfile for CPU too

* Finish the build infrastructure

* Remove extra file

* Comment out the Jenkinsfile section since it is not ready

* Add missing license headers

* Update to newer cudnn that anaconda packaged

* Bump the build numbers for the newer cudnn

* Bring back the toolchain option with a tweak for cuda

* Cache some large packages in the docker and update to llvm 7.0.0

* Merge all the python packages together

* First fix for the conda cuda builds (again)

* Use the tarball version of cudnn since tvm has trouble detecting the other one

* Use llvm 8.0 from the numba packages

* Also use llvm 8.0 for the cpu builds

* Don't use the anaconda compiler for OS X

* Enable Metal on OS X builds

* Make sure to detect undefined variables in scripts

* Fix build when not using cuda
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants