elliottower commented 1 year ago

Description

Uses exact same matplotlib rendering but displays in an interactive pygame window. Added new render_mode argument to Window, gives warning that it should be set but defaults to matplotlib (for backwards compatibility). Added parser argument to manual_control.py to pass in render_mode.

Update: switched env internal render_mode="human" to use pygame, removed utils.window as it was used only for matplotlib rendering, added optional argument to base MiniGridEnv to take "screen_size" for dynamic scaling of rendered content (e.g., larger or smaller display sizes), Added background and unscaled text to pygame window (naive solution leads to pixelated text), updated manual control script controls slightly (added tab to pick up item and shift to drop item, as my laptop doesn't have page up/page down), reworked scripts_test to work with the new changes and mocker test that pygame was closed.

Fixes # (issue): N/A

Type of change

Please delete options that are not relevant.

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[ ] This change requires a documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Rendering using matplotlib backend and pygame (original solution, doesn't resize well and uses unnecessary library):

Rendering using pygame only (full view):

Rendering using pygame only (agent view, scaled game rendering to fill pygame window):

Rendering using pygame only (full view, smaller BabyAI grid env):

Rendering using pygameo nly (agent view, smaller BabyAI grid env):

Checklist:

[x] I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
[x] I have commented my code, particularly in hard-to-understand areas
[ ] I have made corresponding changes to the documentation
[x] My changes generate no new warnings
[x] I have added tests that prove my fix is effective or that my feature works
[x] New and existing unit tests pass locally with my changes

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated
minigrid	✅ Ready (Inspect)	Visit Preview	💬 Add your feedback	Feb 27, 2023 at 5:01AM (UTC)

pseudo-rnd-thoughts commented 1 year ago

What is the performance difference between matplotlib and pygame in terms of frames per seconds?

elliottower commented 1 year ago

What is the performance difference between matplotlib and pygame in terms of frames per seconds?

I'll run some tests for exact performance numbers but it runs very fast with pygame and feels responsive to play.

pseudo-rnd-thoughts commented 1 year ago

Could you make a table with probably 5+ environments of different sizes with the original performance, new performance and percentage difference using the benchmark script https://github.com/Farama-Foundation/Minigrid/blob/master/minigrid/benchmark.py

Also you need to include pygame in the setup as currently it is not installed in the testing

elliottower commented 1 year ago

Could you make a table with probably 5+ environments of different sizes with the original performance, new performance and percentage difference using the benchmark script https://github.com/Farama-Foundation/Minigrid/blob/master/minigrid/benchmark.py

Also you need to include pygame in the setup as currently it is not installed in the testing

Good catch about setup.py, would it not be necessary to have the try catch statements about matplotlib and now pygame if it's included in setup? I see matplotlib isn't included in setup even though it's the default rendering option, maybe they both should be added to simplify the code?

elliottower commented 1 year ago

Didn't get a chance to calcualte the percentages but below are some numbers for 5 different environments of varying sizes, the agent view seems to be very similar but the rendering FPS was higher with matplotlib for most of them. I had a weird issue where when I tried to run the benchmarking with matplotlib in terminal, using the same venv and command I got this error:

python minigrid/benchmark.py --env MiniGrid-DoorKey-5x5-v0 --display-mode matplotlib --num-resets 200 --num-frames 1000                                           (base) 2289ms  Sun Feb 26 23:48:15 2023
Env reset time: 0.3 ms
Rendering FPS : 1278
Agent view FPS: 1305
Traceback (most recent call last):
  File "/Users/elliottower/Documents/GitHub/Minigrid/minigrid/benchmark.py", line 146, in <module>
    benchmark_manual_control(
  File "/Users/elliottower/Documents/GitHub/Minigrid/minigrid/benchmark.py", line 54, in benchmark_manual_control
    env = ManualControl(
TypeError: __init__() got an unexpected keyword argument 'display_mode'

Running with pycharm run scripts works fine (maybe it has to do with adding the source/root paths to pythonpath? those options are checked in the run scripts), but with matplotlib the frames show up in the scientific view automatically and most of them are completely blank graphs, which makes me think there is some sort of rendering issue. I couldn't get an actual interactive window to pop up from within pycharm, so I couldn't test that directly. Maybe someone who knows more about the repo can test my code and see if the blank rendering issue happens for them or what the FPS looks like when run in console rather than pycharm.

Results

I did these manually but if there needs to be multiple re-tests I'll write a bash script or something to run them all.

MiniGrid-LavaGapS7-v0 Pygame (render_mode="RGB_array") Env reset time: 0.4 ms Rendering FPS : 634 Agent view FPS: 1004

Matplotlib (render_mode="RGB_array") Env reset time: 0.3 ms Rendering FPS : 796 Agent view FPS: 1038

Pygame manual_control (render_mode="human") Env reset time: 43.0 ms Rendering FPS : 26 Agent view FPS: 26

Matplotlib manual_control (render_mode="human") Env reset time: 135.3 ms Rendering FPS : 35 Agent view FPS: 25

MiniGrid-PutNear-8x8-N3-v0 Pygame (render_mode="RGB_array") Env reset time: 0.5 ms Rendering FPS : 770 Agent view FPS: 1227

Matplotlib (render_mode="RGB_array") Env reset time: 0.6 ms Rendering FPS : 679 Agent view FPS: 1004

Pygame(render_mode="human") Env reset time: 52.1 ms Rendering FPS : 24 Agent view FPS: 26

Matplotlib(render_mode="human") Env reset time: 139.4 ms Rendering FPS : 34 Agent view FPS: 25

MiniGrid-PutNear-6x6-N2-v0 Pygame (render_mode="RGB_array") Env reset time: 0.4 ms Rendering FPS : 902 Agent view FPS: 1033

Matplotlib (render_mode="RGB_array") Env reset time: 0.5 ms Rendering FPS : 964 Agent view FPS: 1232

Pygame (render_mode="human") Env reset time: 52.5 ms Rendering FPS : 28 Agent view FPS: 25

Matplotlib (render_mode="human") Env reset time: 49.6 ms Rendering FPS : 26 Agent view FPS: 24

MiniGrid-DoorKey-16x16-v0 Pygame (render_mode="RGB_array") Env reset time: 0.5 ms Rendering FPS : 299 Agent view FPS: 1087

Matplotlib (render_mode="RGB_array") Env reset time: 0.4 ms Rendering FPS : 341 Agent view FPS: 1149

Pygame (render_mode="human") Env reset time: 48.5 ms Rendering FPS : 22 Agent view FPS: 23

Matplotlib (render_mode="human") Env reset time: 156.6 ms Rendering FPS : 25 Agent view FPS: 19

MiniGrid-DoorKey-5x5-v0 Pygame (render_mode="RGB_array") Env reset time: 0.4 ms Rendering FPS : 1304 Agent view FPS: 1099

Matplotlib (render_mode="RGB_array") Env reset time: 0.4 ms Rendering FPS : 1316 Agent view FPS: 1287

Pygame (render_mode="human") Env reset time: 42.8 ms Rendering FPS : 23 Agent view FPS: 24

Matplotlib (render_mode="human") Env reset time: 129.1 ms Rendering FPS : 34 Agent view FPS: 25

pseudo-rnd-thoughts commented 1 year ago

I wouldn't worry about testing render_mode="human" Looking at the performance, there doesn't seem to be a significant performance difference between matplotlib and pygame

elliottower commented 1 year ago

I wouldn't worry about testing render_mode="human" Looking at the performance, there doesn't seem to be a significant performance difference between matplotlib and pygame

Okay sounds good. I’m not 100% sure the RGB_array rendering is actually different between the two methods, I can debug it today to check for sure but iirc the only difference was in the show img method and in the window class. Once the rendered frame is generated by matplotlib internally, it gets sent to RGB values and displayed with pygame. So without displaying it using pygame I think it’s the same. It is a bit of a hacky solution as opposed to doing rendering entirely within pygame, but I figured keeping all the graphics the same was probably ideal.

I introduced a new display_mode argument to the window class to choose between pygame and matplotlib (render mode is used elsewhere and would be confusing) but it’s a little convoluted, maybe it’s best to just replace the interactive matplotlib with pygame? Again, I haven’t been able to get the interactive matplotlib working properly with my environment as it just saves each frame to the scientific view, so I can’t really compare directly.

pseudo-rnd-thoughts commented 1 year ago

Ok, so changing matplotlib's back engine doesn't improve performance and has a minor change in what human rendering looks like. Is that correct?

elliottower commented 1 year ago

Ok, so changing matplotlib's back engine doesn't improve performance and has a minor change in what human rendering looks like. Is that correct?

It’s the reverse of that, I’m keeping the backend the same and saving the rendered output to raw RGB values, and then displaying that within Pygame. Doing this doesn’t require changing anything about matplotlib’s backend, although it could hypothetically be gotten rid of altogether and have all the rendering done in pygame

elliottower commented 1 year ago

Tests pass locally but I got these warnings, not sure if they're a problem or not.

====================================================================================== warnings summary ====================================================================================== ../../../anaconda3/lib/python3.9/site-packages/gymnasium/envs/registration.py:521 /Users/elliottower/anaconda3/lib/python3.9/site-packages/gymnasium/envs/registration.py:521: UserWarning: WARN: Overriding environment MiniGrid-BlockedUnlockPickup-v0 already in registry. logger.warn(f"Overriding environment {new_spec.id} already in registry.")

../../../anaconda3/lib/python3.9/site-packages/gymnasium/envs/registration.py:521 /Users/elliottower/anaconda3/lib/python3.9/site-packages/gymnasium/envs/registration.py:521: UserWarning: WARN: Overriding environment MiniGrid-LavaCrossingS9N1-v0 already in registry. logger.warn(f"Overriding environment {new_spec.id} already in registry.")

pseudo-rnd-thoughts commented 1 year ago

Don't worry about that, this is due to pycharm adding the project path when you run python (I believe)

pseudo-rnd-thoughts commented 1 year ago

I believe you need to add an entry_points and update the dockerfile, see the gymnasium dockerfile on the last line https://github.com/Farama-Foundation/Gymnasium/blob/main/bin/docker_entrypoint

elliottower commented 1 year ago

I believe you need to add an entry_points and update the dockerfile, see the gymnasium dockerfile on the last line https://github.com/Farama-Foundation/Gymnasium/blob/main/bin/docker_entrypoint

Good call, thanks. Moved them to bin to make it the same as gymnasium (seems cleaner as well not having as many files in root but I can move them back if we want).

Farama-Foundation / Minigrid

Add pygame rendering support #313

Description

Type of change

Screenshots

Checklist:

Results