Skip to content

Adding docker support for pyro#648

Merged
jpchen merged 1 commit intopyro-ppl:devfrom
neerajprad:dockerfile
Dec 23, 2017
Merged

Adding docker support for pyro#648
jpchen merged 1 commit intopyro-ppl:devfrom
neerajprad:dockerfile

Conversation

@neerajprad
Copy link
Copy Markdown
Member

@neerajprad neerajprad commented Dec 22, 2017

This adds docker support for Pyro. This makes it simple to build Pyro over PyTorch using different configurations:

  • released conda/PyPi packages vs. building both from source using any git branch.
  • using python 2/3
  • using CPU/CUDA.

The Makefile has some common recipes to specify how to build the image. The documentation on building docker images and running them is in the readme. It also makes it easy to run notebook or ipython terminal using the built image. To handle data sharing between the container and the user's host system, we use the same uid/guid while building the image and have a shared workspace that can be configured from the Makefile. This is to make it easy to share data / view results, etc.

Tested locally using different configurations. Since I have refactored this quite a bit, I will continue to test that things are working before getting this merged.

TODO:

While building PyTorch from source using CUDA 9, we get a few intermediate errors even though the build is finally successful, and functional. This still needs a bit more investigation.

Fixes #195

Copy link
Copy Markdown
Member

@fritzo fritzo left a comment

Choose a reason for hiding this comment

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

Looks good, though I'm a docker novice.

Ready to merge?

@neerajprad
Copy link
Copy Markdown
Member Author

Ready to merge?

I did a bunch of refactoring, so running a few quick tests. We should be good to merge after that.

@neerajprad
Copy link
Copy Markdown
Member Author

@fritzo - Thanks for reviewing. We can merge this pending @jpchen's approval.

@rohitsingh0812, @OptimusLime - Please feel free to use / modify this for your use case, and let me know what can be improved.

@jpchen
Copy link
Copy Markdown
Member

jpchen commented Dec 22, 2017

will test cpu please hold off on merging until then, thanks

Copy link
Copy Markdown
Member

@jpchen jpchen left a comment

Choose a reason for hiding this comment

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

lgtm

@jpchen jpchen merged commit 92f77bb into pyro-ppl:dev Dec 23, 2017
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