Render API

[younik]

New render API

Following this discussion: #2540

This PR extends the render() method, allowing the user to specify render_mode during the instantiation of the environment. The default value of render_mode is None; in that case, the user can call render with the preferred mode as before. In this way, these changes are backwards compatible.
In case render_mode != None, the argument mode of .render() is ignored.
In case render_mode = "human", the rendering is handled by the environment without needing to call .render().
With other render modes, .render() returns the results as before. We also introduce _list mode that returns a List with all the renders since the last .reset()or .render(). For example, with render_mode = "rgb_array_list", .render() returns a List of np.ndarray, while with render_mode = "ansi" a List[str].

TODO

• Add deprecation warnings to mode arg in .render() and VideoRecorder

Examples

import gym

env = gym.make('FrozenLake-v1', render_mode="human")
env.reset()
for _ in range(100):
    env.step(env.action_space.sample())
    # env renders automatically, no needs to call .render()
    
env.render()
> None

import gym

env = gym.make('CartPole-v1', render_mode="rgb_array_list")
env.reset()

for _ in range(10):
    env.step(env.action_space.sample())

frames = env.render()
type(frames)
> <class 'list'>
len(frames)
> 11
len(env.render()) # expect 0 because frames are popped by previous .render() call
> 0

env.reset()
len(env.render())
> 1

Example of backward compatibility:

import gym

env = gym.make('FrozenLake-v1')  # default render_mode=None
env.reset()
for _ in range(100):
    # no rendering handled by the environment since render_mode = None
    env.step(env.action_space.sample()) 
    env.render()  # render with human mode (default)
   

[pseudo-rnd-thoughts]

This looks generally good, the only thing this is missing is a test that runs all of the environment with all of these rendering modes. Could you add this?

[younik]

@pseudo-rnd-thoughts thanks for the comments; I added the tests for rendering

[DDDOH]

I can't run the example given in comment, and I'm sure my gym version is the latest (0.24.1), the error message is FrozenLakeEnv.__init__() got an unexpected keyword argument 'render_mode'. Will the document get updated so that users can know clearly what's happening with the render API? Thanks

[younik]

I can't run the example given in comment, and I'm sure my gym version is the latest (0.24.1), the error message is FrozenLakeEnv.__init__() got an unexpected keyword argument 'render_mode'. Will the document get updated so that users can know clearly what's happening with the render API? Thanks

The new render API is not in the release version yet, if you want to use it, you should install gym from source

[DDDOH]

That makes sense. Looking forward to the formal released version.

[wookayin]

Why did the authors decide to change the render API -- the removal of argument render_mode -- this will break all existing codes that would use offline rendering (render("rgb_array")). The core gym.Env interface has mode (or render_mode) as in https://github.com/openai/gym/blob/master/gym/core.py#L185, which shall not be removed until v1.0 after enough deprecation period.

In addition, there seems many bugs around: As discussed in the previous comments and #2825, render_mode constructor is not even properly implemented (which is already fixed in the HEAD). Also it returns a different type/shape (which I think is wrong) than what would be in the previous versions. (Update: I've filed an issue for this: #2889)

I think despite the new default/API, we could maintain the backward compatibility. Could you please reconsider adding it back unless the breaking change and removal of the argument is really necessary?

[younik]

Why did the authors decide to change the render API -- the removal of argument render_mode -- this will break all existing codes that would use offline rendering (render("rgb_array")). The core gym.Env interface has mode (or render_mode) as in https://github.com/openai/gym/blob/master/gym/core.py#L185, which shall not be removed until v1.0 after enough deprecation period.

Hello, for a detailed explanation of why this render API was introduced see #2540
We are aware this is a breaking change, thus as you pointed out we introduced a deprecation warning that will be there for enough period. For now, the code is backwards compatible.

In addition, there seems many bugs around: As discussed in the previous comments and #2825, render_mode constructor is not even properly implemented (which is already fixed in the HEAD). Also it returns a different type/shape (which I think is wrong) than what would be in the previous versions. (Update: I've filed an issue for this: #2889)

The constructor is properly implemented in source; as I answered in the issue, we mistakenly merge the update documentation before the release, sorry about that. Also, #2889 is the expected behaviour, I will answer your issue soon.

[wookayin]

If the maintainers intended such a breaking change, this should be done in v1.0 release or after some enough deprecation/announcement period. I feel release of this new API went a bit hasty without enough discussion. I would argue that we should keep the backward compatibility as much as possible. As a minority opinion, with all due respect I don't think the new API is necessarily better --- I understand the purpose and intention discussed since #2540 but they could've been implemented in a different way without hurting the sensible interface design and backward compatibility.

All that said, I do understand that in some cases breaking changes and radical changes are necessary. However, this should be done rather carefully with all other relevant changes together. For example, it is said (#2891) render(**kwargs) are deprecated and will be removed at gym v1.0 --- but what's the point of doing this if you have already broke the API and suddenly changed the behavior in v0.24.0? They could have done together so it can be fool-proof preventing all the unwanted use cases by normal users who would never have been aware of the change.

[younik]

they could've been implemented in a different way

Even if it had already been merged, I would be sincerely happy to implement a better proposal that will address the problems explained in #2540

we should keep the backward compatibility
if you have already broke the API

What do you mean? Legacy codes still work (with just a warning), the current changes are backward compatible

[wookayin]

The current behavior and API is different from older versions in the following sense. I would appreciate if you can explain why you would think this is backward compatible. The changes are "on purpose", so in my view it is obvious that we broke backward compatibility:

>>> env.render('rgb_array')
Wrapper.render() takes 1 positional argument but 2 were given

This change is perhaps OK, because it throws error rather than behaving wrongly. Users can straightforwardly switch from positional arguments to (forced) keyword arguments (at the acceptable cost of code migration). Or we could improve the wrapper to pass *args as well as **kwargs.

The real problem is when render_mode = "rgb_array" (Note: UPDATED the code per the new API change #2671):

>>> env = gym.make(..., render_mode="rgb_array")
>>> env.render()

• Now returns a list, as explained/proposed in this PR Render API #2671
• Previously this would return a (single) rendered frame of w.r.t the current state (see Fix: add mujoco render arguments to init #2891)

>>> env.render(width=300, height=300)

The additional parameters to renderer seems buggy at this point (#2891), so I can't tell right now whether it will behave the same (i.e. overriding the rendering options specified in the constructor when provided). But I can guess those keyword arguments won't have any effects, especially when no errors are thrown and keyword arguments are silently ignored: currently mode is ignored, and #2891 says

Because the frames are computed before the render call;

so those arguments to render(...) will be also ignored, as all the rendering related options are specified at the instance-level only.

As I mentioned in another thread (#2889), we should have a different rendering mode for the multiple-frames-since-the-last-call-for-frame-skips behavior other than rgb_array, for instance, rgb_arrays.

[wookayin]

I looked at the code and figured out what @younik meant by backward compatibility.

>>> env = gym.make("HalfCheetah-v4")
>>> env.reset();
>>> env.render(mode='rgb_array', width=300, height=300).shape  
(480, 480, 3)

The only change is that the env wrapper requires keyword argument (no positional arguments, i.e., env.render("rgb_array")). The behavior is backward compatible with the default behavior not having changed. My apologies I didn't look at the changes/codes very carefully and got it wrong. So it turns out that the new versions of gym would make, fortunately, no big disasters for existing codes, except for a very straightforward signature change.

However, I think render_mode="rgb_array" still needs to behave exactly same as if the same mode=... parameter were given to each env.render() call. This would be more consistent and reasonable behavior; can we consider having a different name for render_mode?

[younik]

@wookayin sorry, I was answering you, but your message make me realize that the mode as argument was broken and I worked for the fix (#2893)

So, all old-style code work as before.

However, I think render_mode="rgb_array" still needs to behave exactly same as if the same mode=... parameter were given to each env.render() call. This would be more consistent and reasonable behavior; can we consider having a different name for render_mode?

In general, I think we want to encourage the use of rgb_array over single_rgb_array since not every environment would support it.
Since one should be aware of the new render API to use it; this should not introduce confusion. We can obviously discuss it; rgb_array and single_rgb_array vs multiple_rgb_array and rgb_array doesn't make too much different.
But as I said, the standard behaviour should be returning a collection of frames.

[wookayin]

@younik @pseudo-rnd-thoughts I still think the new API introduced here doesn't make much sense. For the sake of consistency, this should have been renamed as something like rgb_arrays or rgb_array_list when used in the constructor. It is extremely confusing that env.render(mode="rgb_array") returns a single RGB frame but Env(mode="rgb_array").render() returns a list -- which is even stateful.

Any chance to revisit this? This is perhaps the only, last time to have it fixed before the breaking change. Hope this can be addressed before v0.26.0 release (#3056).

UPDATE: This already has been addressed, in #3040 (unreleased yet). Per the release note v0.26.0 (#3056),

Render modes - In v25, there was a change in the meaning of render modes, i.e. "rgb_array" returned a list of rendered frames with "single_rgb_array" returned a single frame. This has been reverted in this release with "rgb_array" having the same meaning as previously to return a single frame with a new mode "rgb_array_list" returning a list of rgb arrays. @pseudo-rnd-thoughts

I'm supportive of this change. For a record/historic purpose, let me put a pointer here for those who might find this issue in the future.