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

Add wrappers.vector.HumanRendering #1013

Conversation

pseudo-rnd-thoughts
Copy link
Member

Description

Rather than add human rendering to every vector environment, this wrapper hopes to add human style rendering as a wrapper for environments to use.
Completes #880 and fixes cartpole rendering in #872

# Conflicts:
#	gymnasium/envs/classic_control/cartpole.py
#	gymnasium/wrappers/rendering.py
# Conflicts:
#	gymnasium/vector/vector_env.py
@pseudo-rnd-thoughts
Copy link
Member Author

@TimSchneider42 Could you test this PR and see if it works for you?

@TimSchneider42
Copy link
Contributor

I had to make an adjustment to make it work:

diff --git a/gymnasium/wrappers/vector/rendering.py b/gymnasium/wrappers/vector/rendering.py
index c4f691a44..1d7463152 100644
--- a/gymnasium/wrappers/vector/rendering.py
+++ b/gymnasium/wrappers/vector/rendering.py
@@ -131,7 +131,9 @@ class HumanRendering(VectorWrapper):
             assert (cols * subenv_size[0] * scaling_factor == self.screen_size[0]) or (
                 rows * subenv_size[1] * scaling_factor == self.screen_size[1]
             )
-            return rows, cols, scaling_factor
+            self.num_rows = rows
+            self.num_cols = cols
+            self.scaled_subenv_size = (int(subenv_size[0] * scaling_factor), int(subenv_size[1] * scaling_factor))
 
             assert self.num_rows * self.num_cols >= self.num_envs
             assert self.scaled_subenv_size[0] * self.num_cols <= self.screen_size[0]

Apart from this, it worked fine.

One thing that is confusing to me is that human rendering can no longer be enabled with the render_mode argument but rather requires the user to apply the wrapper manually. We could consider wrapping the classic control environments automatically if the render mode is human.

Also, is there a reason why this wrapper defines the render function to always return none? We could just leave it undefined so that the user can still call the original render function if they require a RGB image.

Best,
Tim

@pseudo-rnd-thoughts pseudo-rnd-thoughts merged commit d196497 into Farama-Foundation:main Apr 23, 2024
11 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants