-
Notifications
You must be signed in to change notification settings - Fork 4.4k
[containers] Enables container support for scenes that use visual obsvervations #547
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
Conversation
mmattar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just feedback on diffs ... will try running it tomorrow.
| '--port', str(self.port), | ||
| '--seed', str(seed)]) | ||
| else: | ||
| docker_ls = ("exec xvfb-run --auto-servernum" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A comment explaining docker_ls would be very helpful here, particularly with regards to the dimensions (640x480x24).
docs/Using-Docker.md
Outdated
| # Using Docker For ML-Agents (Experimental) | ||
|
|
||
| We currently offer an experimental solution for Windows and Mac users who would like to do training or inference using Docker. This option may be appealing to those who would like to avoid installing Python and TensorFlow themselves. The current setup forces both TensorFlow and Unity to _only_ rely on the CPU for computations. Consequently, our Docker support is limited to environments whose agents **do not** use camera-based visual observations. For example, the [GridWorld](Learning-Environment-Examples.md#gridworld) environment is **not** supported. | ||
| We currently offer a solution for Windows and Mac users who would like to do training or inference using Docker. This option may be appealing to those who would like to avoid installing Python and TensorFlow themselves. The current setup forces both TensorFlow and Unity to _only_ rely on the CPU for computations. Consequently, our Docker simulation [uses the CPU](https://en.wikipedia.org/wiki/Xvfb) to do visual rendering. This means that environments which involve agents using camera-based visual observations might be slower. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Linking to https://en.wikipedia.org/wiki/Xvfb in "uses the CPU" was a bit counter-intuitive for me.
- The very end of this sentence "might be slower", presumably you mean training and inference speeds. I would clarify that. (Optional: is there anything we can say regarding how much slower we expect it to run?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Linking to https://en.wikipedia.org/wiki/Xvfb in "uses the CPU" was a bit counter-intuitive for me.
Xvfbis a virtualized XServer which does not use the GPU, would you prefer if the text hyperlinked saiddoes not use the GPU?
The very end of this sentence "might be slower", presumably you mean training and inference speeds. I would clarify that. (Optional: is there anything we can say regarding how much slower we expect it to run?)
I mean training. Although you could very well use the container for inference as well, it seems more designed for a training use case. I will clarify there.
We are not significantly slower for the environments we support currently. Having said that, we are using the CPU to do computation which is typically done in a GPU. So when we do get to more complex and rich rendering scenes, I expect performance to drop. That phrase was mostly added to ward off potential issues popping up on Github in the mean time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if we're going to link to Xvfb then we should probably introduce it here too.
docs/Using-Docker.md
Outdated
| @@ -1,6 +1,7 @@ | |||
| # Using Docker For ML-Agents (Experimental) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Remove "(Experimental)"? If so, then also remove it from the Repo Homepage (features section).
- (Minor) In this page we used 3D Ball as the running example for container and env names. But now the image "docker_build_settings.png" links to the Gridworld scene. I think we ought to either update the naming conventions or have the image point to 3Dball, just for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both good points. Will fix.
|
Would it be possible to provide a lightweight container for only doing vector observations, and this new one for visual observations? I think if it isn't necessary, we should direct people to use headless mode and the slimmer image. |
|
@awjuliani I am not sure I understand what slimmer or lightweight mean in this context. If you mean that there are more lines in the DockerFile, it is because we are taking content from the previous base image. The only changes from the previous container is that we are using Ubuntu and installing xvfb. As far as headless is concerned, someone who wants to use only vector observations can very well build a headless Unity build and use it with our current setup. I am happy to add a note on that. |
|
When trying out docker image from develop-feature-docker-improvements I get the following error. Debug info from gdb: mono_gdb_render_native_backtraces not supported on this platform, unable to find gdb or lldb Got a SIGABRT while executing native code. This usually indicates Traceback (most recent call last): I run it on a Mac. |
|
Hi @Lakrix, Some useful information would be:
|
45071b7 to
c8130ba
Compare
mmattar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One additional comment on the older version of this file .... in the "docker run --name 3DBallContainer.first.trial \ ..." command - the environment name should be changed from "3Dball" to "3DBall".
docs/Using-Docker.md
Outdated
|
|
||
| **NOTE** If you are only collecting vector observations from Unity, you can select the `headless` option here: | ||
|
|
||
|  |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a difference between the *_headless.png and *_noheadless.png images, except that headless points to the GridWorld scene, while noheadless points to the 3DBall scene.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was a typo. I think I didn't commit the updated file. The _headless.png should be identical to the _noheadless.png except for the former having headless checked.
docs/Using-Docker.md
Outdated
|
|
||
| Then click `Build`, pick an environment name (e.g. `3DBall`) and set the output directory to `unity-volume`. After building, ensure that the file `<environment-name>.x86_64` and subdirectory `<environment-name>_Data/` are created under `unity-volume`. | ||
|
|
||
| **NOTE** If you are only collecting vector observations from Unity, you can select the `headless` option here: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider including the headless commentary in the list above. That why when a user is following the instructions, they better understand what role the headless option represents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"If you are only collecting vector observations from Unity" --> "If the environment does not contains visual observations"
Reason for this change too is that headless presumably is about visual, so if they're using vector and/or text observations then they should be ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I am going to fold the NOTE into the checklist. Let me know if you think the two images for the headless and noheadless options seem useless now?
vincentpierre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modify docs/Readme.md
Merge docs/images/docker_build_settings_headless.png and docs/images/docker_build_settings_noheadless.png into one image with annotation.
|
It works for me feel free, the changes I requested are only around docs |
|
@vincentpierre check new annotated image please. |
0c6bf42 to
0afcc73
Compare
Based on @AdamPalmarUnity's setup. Please take your time to test and try it out.