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

Rendering of CartPoleVectorEnv is broken #1007

Closed
1 task done
TimSchneider42 opened this issue Apr 10, 2024 · 7 comments
Closed
1 task done

Rendering of CartPoleVectorEnv is broken #1007

TimSchneider42 opened this issue Apr 10, 2024 · 7 comments
Labels
bug Something isn't working

Comments

@TimSchneider42
Copy link
Contributor

Describe the bug

Hi,

the rendering of CartPoleVectorEnv is broken in two places:

  1. In line 548, self.clock is assigned instead of self.clocks
  2. In line 561, self.state is used instead of state

Because of these two bugs, CartPoleVectorEnv.render() throws exceptions. I will create a PR in a moment.

Best,
Tim

Code example

No response

System info

No response

Additional context

No response

Checklist

  • I have checked that there is no similar issue in the repo
@TimSchneider42 TimSchneider42 added the bug Something isn't working label Apr 10, 2024
TimSchneider42 added a commit to TimSchneider42/Gymnasium that referenced this issue Apr 10, 2024
TimSchneider42 added a commit to TimSchneider42/Gymnasium that referenced this issue Apr 10, 2024
@TimSchneider42
Copy link
Contributor Author

Just noticed that I overlooked #872, which addresses this issue already but still needs a pull request. Apologies for the duplication! Should I close this issue in favor of #872?

TimSchneider42 added a commit to TimSchneider42/Gymnasium that referenced this issue Apr 10, 2024
@pseudo-rnd-thoughts
Copy link
Member

Thanks, for the issue. My proposal was to actually not contain "human" rendering in the custom vector environment as this is difficult to maintain.
Rather, we should have a wrappers.vector.HumanRendering for which all "human" vector rendering is computed through.
This still requires rgb array rendering in the vector environment but this feels like a simpler solution for most vector environments.

Thoughts @TimSchneider42 and maybe @Kallinteris-Andreas or @RedTachyon

@TimSchneider42
Copy link
Contributor Author

I like the idea of solving human rendering with a wrapper. At least in this case, I expect it to be easier to implement and maintain, since the rgb_array image simply gets rendered by pygame.

@pseudo-rnd-thoughts
Copy link
Member

Agreed. I have been thinking about how to implement the vector human rendering but I haven't come to a satisfactory general solution.
If you have any idea, I would love to hear them

@TimSchneider42
Copy link
Contributor Author

I think for most environments we could simply render in "rgb_array" mode and then have the wrapper you proposed render all of them side by side in a single window with pygame. For more sophisticated environments, like this one, though, there should still be an option for custom rendering solutions, like just showing the simulator interface without having to render RGB images.

In general, I think that human rendering should be detached from RGB rendering in the API. I just wrote a small proposal about that in #1010.

@RogerJL
Copy link
Contributor

RogerJL commented Jun 1, 2024

Can this be closed now?

@TimSchneider42
Copy link
Contributor Author

Yes, it is fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants