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

Docker support would be great #3

Open
stevenw opened this issue Nov 8, 2018 · 19 comments
Open

Docker support would be great #3

stevenw opened this issue Nov 8, 2018 · 19 comments

Comments

@stevenw
Copy link

stevenw commented Nov 8, 2018

No description provided.

@machinaut
Copy link

Could you describe the use case or what features you'd like that are missing?

@stevenw
Copy link
Author

stevenw commented Nov 9, 2018

An official Dockerfile should allow much easier installation and should be easier to be platform agnostic.

The current installation documentation not only have its own platform specific instructions but also has multiple links to another project's installation documentation, all of which has their own platform specific instructions.

As a windows user and AI newbie I was turned off trying to run the code from Spinning Up due to the installation process.

@xeb
Copy link

xeb commented Nov 9, 2018

One challenge is going to be the MuJoCo license & installation. Some of the required files (e.g. in ~/.mujoco) can be mapped with a volume. If you will accept a PR for a Dockerfile, that would be appreciated. If not, we can create a spinningup-docker repo as an example. I'm interested in helping make this work.

@machinaut
Copy link

I think this would be a good solution if you could run all of the code in it.

@xeb I'm curious how you would get the plotting or rendering functionality to work. In the past it has been difficult to get docker containers to render graphical interactive windows.

@xeb
Copy link

xeb commented Nov 12, 2018

I was hoping x11vnc, but having issues with pyglet. Did a sample over the weekend. I can at least run experiments. Will add more to the README, but wanted to share progress: https://github.com/xeb/spinningup-docker

@machinaut
Copy link

@xeb keep us updated! If it ends up working then it would be a good thing to have in the repo!

@gvgramazio
Copy link

I'm interested in contributing in this feature. What are the requirements of the feature?

@machinaut
Copy link

I think I would be most interested in merging something that allowed full functionality via docker (instead of only partial).

The tricky part here is getting the plotting to function as expected.

@gvgramazio
Copy link

gvgramazio commented Dec 11, 2018

The only thing I can't test is the MuJoCo license because I don't have any. If it's fully functional except for the MuJoCo license (that could be implemented by anyone that have it) then are you interested?
Could you explain better the plotting functionality? Are you interested in plotting or in visualizing the plot directly in the container? The Docker container should have support for any other library/tool not explicitly listed as required?

@cirocavani
Copy link

I create a Dockerfile that "pass" the installation test (includes the plot tool, but no MuJoCo). It also uses NoVNC javascript VNC client to render X on a browser directly.

https://github.com/cirocavani/reinforcement-learning/tree/master/openai-spinningup

@gvgramazio
Copy link

@cirocavani I've taken a look at your repo. There are a few suggestions:

  • You should never use things like upgrade, dist-upgrade and similar. One of the major points of using Docker, that is have a control on the version of packages used, would be wasted.
  • There is no point in using conda in a Docker container. You are not going to mix python versions.
  • There are better ways of importing git repositories. The way you are using is wrong. Either specify the commit or add a random variable to your image before the download. Right now, if there is any change in the git repo, your image will be not up to date.

@gvgramazio
Copy link

@cirocavani I forgot another suggestion: do not use sudo inside Docker images.

@gvgramazio
Copy link

I've written a basic dockerfile with the requirements stated so far (is capable of running examples and plotting results) to be included in the main project. See my fork to try it.

In order to work with it, simply build the image:

sudo docker build -t spinningup .

Adjust the permissions of xhost:

xhost +local:root

Run the container exposing the xhost so that container can render to the correct display by reading and writing through the X11 Unix socket:

sudo docker run -it --rm \
  -e DISPLAY=$DISPLAY \
  -v /tmp/.X11-unix:/tmp/.X11-unix \
  spinningup

Enjoy your container. Of course, you can plot results.

When have you done with it, adjust again the permissions of xhost:

xhost -local:root

Note that I'm writing this post in order to receive a feedback and feature requests. I don't consider it ready for a pull request yet.

I will be satisfied by adding the support for python (currently works only with python3) and managing in a better and safer way the x11 redirection. Both things should be done by tomorrow. Feel free to ask for more features.

@machinaut
Copy link

December is almost over (at least from a work-week perspective)! @cirocavani thanks for the code and I'll have time to look at it in the new year.
Really appreciate your hard work on this one!

@gvgramazio
Copy link

In my case, holidays are the only period when I can work on this. :)

@machinaut, have you thought about other features that the docker image should have? @cirocavani inserted support for Jupyter. Is that required? In that case, a good thing could be to insert the support for gym video output inside Jupyter.

Also, one thing that changes a lot the docker image is if you prefer an image when one can run the library and exit or if you want it in a sort of standalone application that allows to run it to produce results to be automatically stored in the host system. In my point of view, the first one is the right choice. The second one can be published on dockerhub instead.

@alek5k
Copy link

alek5k commented Feb 6, 2019

Hello all,

I decided I wanted to work on a virtual machine rather than using docker (to avoid issues with x11 and xhost, and to leverage GPU if needed), so I wrote a bash setup script that installs SpinningUp and all dependencies from scratch. You can find it here:

https://gist.github.com/alek5k/dbd6c7235c46584bd25221c52f27d308

The script is mostly unattended, you just need to enter sudo user and pass once to install apt packages.
It makes some assumptions for where you want to set up your folders, but i've flagged that as a 'todo' item to make it configurable in the script if it becomes popular.

I added this here in-case this information can be used in future for the 'automation' aspect of setting up SpinningUp (not limited to docker, perhaps for use with vagrant, AWS/Azure VMs, etc)

I tested it on a clean Ubuntu 18.04.1 x64 virtual machine.

@gvgramazio
Copy link

You can both use x11 and GPU with docker (as long as you have an Nvidia). Working on a docker container instead of a virtual machine could be better for performance, portability and testing. These are the reasons why a docker support would be great. Unfortunately I hadn't a feedback from the developers of spinningup in the last month.

@kata314
Copy link

kata314 commented Oct 15, 2019

@gvgramazio, is your approach only working with docker running on a linux host? I can't plot or display the results of PPO in the LunarLander-v2.
Specifically, python3 -m spinup.run plot /spinningup/data/installtest/installtest_s0 breaks with:

_tkinter.TclError: couldn't connect to display "/private/tmp/com.apple.launchd.uuzCe1hm7B/org.macosforge.xquartz:0"

@gvgramazio
Copy link

@thomasarvanitidis Yes, it's supposed to work only on a linux host. Sorry about that.

ppiech pushed a commit to ppiech/spinningup that referenced this issue Jan 23, 2020
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Mar 28, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Mar 28, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Mar 28, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Mar 28, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Mar 28, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Apr 13, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Sep 26, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Sep 26, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Sep 26, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Sep 26, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Sep 26, 2024
mahdinobar added a commit to mahdinobar/spinningup that referenced this issue Sep 26, 2024
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

No branches or pull requests

7 participants