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

[Proposal] Allow multi-mode rendering for new Render API #3038

Closed
1 task done
YouJiacheng opened this issue Aug 22, 2022 · 8 comments
Closed
1 task done

[Proposal] Allow multi-mode rendering for new Render API #3038

YouJiacheng opened this issue Aug 22, 2022 · 8 comments

Comments

@YouJiacheng
Copy link
Contributor

YouJiacheng commented Aug 22, 2022

Proposal

Optionally(i.e. for some environments) allow Env.render_mode to be a tuple to support rendering in multiple modes.

Motivation

Multi-mode rendering might be useful, e.g. RGBD.
In old Render API, this is trivial, since we can simply call env.render with different modes multiple times.
However, in new Render API, we must use multiple environment replicas with different render_mode.

Alternatively, user can add a new mode for each valid combination, which is not ergonomic.

Checklist

  • I have checked that there is no similar issue in the repo (required)

@younik

@pseudo-rnd-thoughts
Copy link
Contributor

This seems extremely complex to implement with limited use cases.
Why do you want this function?

It could be possible to implement as a wrapper that reconstructed the original environment with a new render mode and on step both environments would be run to get both environment render results

@YouJiacheng
Copy link
Contributor Author

Let me clarify:
I do NOT say that ALL environment must support this functionality. Just allow this behavior to be "legal" in gym API.
Some environments can easily support this functionality, while it might be better to replicate environment for other environments.

@YouJiacheng YouJiacheng changed the title [Proposal] Add multi-mode rendering support for new Render API [Proposal] Allow multi-mode rendering for new Render API Aug 22, 2022
@pseudo-rnd-thoughts
Copy link
Contributor

That makes more sense
But I wrote a short example and this is completely possible to use currently.
The full environment checker will fail and the passive environment checker will raise a warning as this is unexpected.
If you want to ignore the specification for your particular environment you can but I don't think we would want to add this as a default feature

from typing import Union, Tuple, Optional

import gym
from gym.core import ObsType, ActType
from gym.utils.env_checker import check_env


class MultiRenderEnv(gym.Env):

    metadata = {
        "render_modes": ["human", "rgb_array"]
    }

    def __init__(self, render_mode: Union[str, tuple[str]]):
        self.action_space = gym.spaces.Box(0, 1)
        self.observation_space = gym.spaces.Box(0, 1)

        self.render_mode = render_mode

    def reset(
        self,
        *,
        seed: Optional[int] = None,
        options: Optional[dict] = None,
    ) -> Tuple[ObsType, dict]:
        super().reset(seed=seed)
        self.observation_space.seed(seed)
        return self.observation_space.sample(), {}

    def step(
        self, action: ActType
    ) -> Tuple[ObsType, float, bool, bool, dict]:
        return self.observation_space.sample(), 0, False, False, {}


gym.register("multi-render-v0", entry_point=MultiRenderEnv)

env = gym.make("multi-render-v0", render_mode=("human", "rgb_array"))
# check_env(env.unwrapped)  # this will fail

@YouJiacheng
Copy link
Contributor Author

YouJiacheng commented Aug 22, 2022

Could you explain why you don't want to add this as a default feature? The passive checker(make) will raise a warning which is annoying.
Alternatively, do you want to let environment checker(including make) read "render_mode checker" from metadata so that users can customize render_mode arbitrarily. Concretely, metadata["render_mode"] can be a Callable[[Any], bool].

@pseudo-rnd-thoughts
Copy link
Contributor

This seems like a niche feature that minimal users will need. What common cases do you imagine this being used?
Yes, the passive environment checker will raise a warning however this is a warning and can be ignored by the users or they can turn off the passive environment checker when registering the environment.

For changing how the render modes worked, we decided to move from .render(mode) to the mode being determined at initialisation. This was required by Atari environments already and decided it was a good idea to move to Gym.
Therefore the suggested Callable version would be completely different from previous versions.

@RedTachyon
Copy link
Contributor

@YouJiacheng here's the thing about the design philosophy of the core API - it mostly includes things that you generally want all envs to support. So whether you use Atari, Unity, Brax, MuJoCo, you know that you can do env.reset(), or that you can use the rgb_array render mode and then get a numpy array from env.render(). This is necessary to ensure the wide compatibility of training codes, so that if your RL algorithm works on CartPole, then it should also work on Montezuma's Revenge (ignoring performance concerns).

This feature would work for some environments, and not for others, right from the get-go. So it can't be reliably included in the core API.

But here's the thing - if you're developing a custom environment, it actually makes little to no difference for you in the implementation. There's absolutely nothing stopping you from, for example, using render with rgb_array, and then defining a new method called env.render_show() which will perform the human-style rendering. Even in most of the environments included in gym, you can typically find the relevant methods for each type of rendering and call them separately.

@YouJiacheng
Copy link
Contributor Author

YouJiacheng commented Aug 23, 2022

Thanks for nice explanations from @pseudo-rnd-thoughts and @RedTachyon !

Using a custom(maybe private) API of custom environment is definitely better than making the general & public API complicated!

I agree that we should keep the core API simple and minimum. I like this design philosophy - core API only provide minimum guarantee generally supported by all envs(with as less as possible violations), given the fact that the majority of RL algorithms only need this minimum guarantee.

@YouJiacheng
Copy link
Contributor Author

Actually #2989 is one of the use-cases. But it seems that use a wrapper is a better idea.

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

3 participants