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

Added render() to gym.vector environments #1624

Closed
wants to merge 1 commit into from
Closed

Added render() to gym.vector environments #1624

wants to merge 1 commit into from

Conversation

jinyeom
Copy link

@jinyeom jinyeom commented Jul 30, 2019

This PR adds rendering capability to vectorized environments (gym.vector) via env.render(), where the rendered images are tiled to be shown in a single window (the same as in baseline).

gym_vector_env_render

@christopherhesse
Copy link
Contributor

christopherhesse commented Aug 2, 2019

One minor thing that I've run into before, is that, it might be better for render(mode="rgb_array") to return a batch of images rather than a single large image. Related to that, it might be better for render(mode="human") to call `render(mode="rgb_array") and then display the tiled image, and pass through the mode value in other cases, returning a list of the results.

This PR also adds get_images and get_viewer to the VectorEnv API, which doesn't seem necessary to support render.

Thanks for the PR, I think having render on here makes a lot of sense, but I'd like to potentially avoid some unnecessary complexity from the baselines setup if possible and keeping the size of the interface small is very helpful for that. Right now it's the rather large gym.Env interface + num_envs, single_observation_space, single_action_space, closed, viewer, reset_async, reset_wait, step_async, step_wait. closed and viewer should probably not even be public properties.

@jinyeom
Copy link
Author

jinyeom commented Aug 3, 2019

Thanks for the response! Being able to render from a vectorized set of environments is something I found useful for debugging. If it's just a matter of API design, I can make some changes to simplify it. Would that be okay? :)

@Bam4d
Copy link

Bam4d commented Aug 4, 2021

@jkterry1 I think this is a good feature in general but the implementation is possibly problematic as it assumes that all environments can be rendered using the following code, which appears to be environment specific:

from gym.envs.classic_control import rendering
self.viewer = rendering.SimpleImageViewer()

Some environments return images in the form HWC and some return WHC so rendering through this kind of interface might result in garbled images?

A possible solution is a generic fast (like pyglet) renderer for RGB images that can be configured to WHC vs HWC depending on the environment being used (perhaps in the VecEnv wrapper initialization). When vecEnv.render() is called, it renders the tiled environments.

Another potential issue is that in some cases people will run a VecEnv with a large number of environments, and a call to "render" will be super slow, so maybe we need a max number of environments to tile.

Finally, I agree with @christopherhesse in that the two get_images and get_viewer functions are redundant as this can be handled in the render() function.

@jkterry1
Copy link
Collaborator

jkterry1 commented Aug 4, 2021

Thanks a ton @Bam4d. In that case I'm going to close this and we can revisit it after the planned breaking changes to vector environments are made.

@vwxyzjn
Copy link
Contributor

vwxyzjn commented Aug 17, 2021

Hey re-opening this issue. @Bam4d raises a good point that it makes certain assumptions on how the env could be rendered, but I feel this is an acceptable downside. The main thing for me is the speed implication of rendering lots of envs at once, which is not friendly to throughput and memory.

Maybe we can address this with an argument like env_renderable. So env_renderable = lambda env_idx: env_idx == 0 means only the first env is going to be rendered, and env_renderable = lambda env_idx: True would rendered every env in a tiled image?

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.

None yet

6 participants