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

Update Gymnasium to v1.0.0 #1837

Merged
merged 38 commits into from
Nov 4, 2024

Conversation

pseudo-rnd-thoughts
Copy link
Contributor

@pseudo-rnd-thoughts pseudo-rnd-thoughts commented Feb 13, 2024

This PR updates SB3 to Gymnasium v1.0, read the release-notes to see all the changes.

closes #2023

SB3 contrib PR: Stable-Baselines-Team/stable-baselines3-contrib#261
RL Zoo PR: DLR-RM/rl-baselines3-zoo#475

Motivation and Context

Gymnasium is the core API used in SB3, therefore would be helpful for both SB3 to use the latest version and that SB3 provides a great testing ground to check for that the Gymnasium release works as intended.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)

Checklist

  • I've read the CONTRIBUTION guide (required)
  • I have updated the changelog accordingly (required).
  • My change requires a change to the documentation.
  • I have updated the tests accordingly (required for a bug fix or a new feature).
  • I have updated the documentation accordingly.
  • I have opened an associated PR on the SB3-Contrib repository (if necessary)
  • I have opened an associated PR on the RL-Zoo3 repository (if necessary)
  • I have reformatted the code using make format (required)
  • I have checked the codestyle using make check-codestyle and make lint (required)
  • I have ensured make pytest and make type both pass. (required)
  • I have checked that the documentation builds using make doc (required)

Note: You can run most of the checks using make commit-checks.

Note: we are using a maximum length of 127 characters per line

@pseudo-rnd-thoughts
Copy link
Contributor Author

pseudo-rnd-thoughts commented Feb 13, 2024

There are only two main issues to resolve + need to rewrite VecVideoRecorder

  1. As we have removed Env.__getattr__, then we need to update to use Env.get_wrapper_attr
  2. Registration of external environment is not automatical, i.e., atari, therefore a hack was added to fix it

@pseudo-rnd-thoughts
Copy link
Contributor Author

The CI seems to have failed due to reasons unrelated to the version change

@araffin
Copy link
Member

araffin commented Feb 14, 2024

Thanks for the PR =)

As we have removed Env.getattr, then we need to update to use Env.get_wrapper_attr

if possible (and if not too hacky), I would add backward compat changes to handle both gymnasium 0.29 and 1.x.

@pseudo-rnd-thoughts
Copy link
Contributor Author

Thanks for the PR =)

No worries, all the errors seem expected and no unexpected bugs are found

As we have removed Env.getattr, then we need to update to use Env.get_wrapper_attr

if possible (and if not too hacky), I would add backward compat changes to handle both gymnasium 0.29 and 1.x.

I believe the changes made should be backward compatible. Just updating VecRecordEnv needs to be fully updated / rewritten

@Kallinteris-Andreas
Copy link
Contributor

Assuming the Environment which used gymnasium==0.29 is not broken from the gymansium==1.0 update, it should just work without any additional compatibility work in SB3

@araffin
Copy link
Member

araffin commented Feb 16, 2024

Assuming the Environment which used gymnasium==0.29 is not broken from the gymansium==1.0 update, it should just work without any additional compatibility work in SB3

I'm talking about allowing people to use 0.29 with SB3.

I believe the changes made should be backward compatible.

mmh, I would double check the getattr() part, I remember it was warning the user

@pseudo-rnd-thoughts
Copy link
Contributor Author

mmh, I would double check the getattr() part, I remember it was warning the user

If the code works with 1.0.0a1 then it will work with 0.29 but possibly not the other way around

@pseudo-rnd-thoughts
Copy link
Contributor Author

@araffin I believe I have fixed all the issues except for updating VecVideoRecorder.
I don't know how SB3 works internals, would you be able to get one of your devs to update that?

@araffin
Copy link
Member

araffin commented Feb 19, 2024

I don't know how SB3 works internals, would you be able to get one of your devs to update that?

Currently, there is only one active dev (me...), Quentin (@qgallouedec ) is helping me with answering questions and doing code reviews, for the rest, we have to rely on the community.

In the meantime, you could try running tests in SB3 contrib and RL Zoo with this branch, that should unveil other bugs/issues.

@Kallinteris-Andreas
Copy link
Contributor

i tested this: (from https://stable-baselines3.readthedocs.io/en/master/guide/examples.html#record-a-video)

import gymnasium as gym
from stable_baselines3.common.vec_env import VecVideoRecorder, DummyVecEnv

env_id = "CartPole-v1"
video_folder = "logs/videos/"
video_length = 100

vec_env = DummyVecEnv([lambda: gym.make(env_id, render_mode="rgb_array")])

obs = vec_env.reset()

# Record the video starting at the first step
vec_env = VecVideoRecorder(vec_env, video_folder,
                       record_video_trigger=lambda x: x == 0, video_length=video_length,
                       name_prefix=f"random-agent-{env_id}")

vec_env.reset()
for _ in range(video_length + 1):
  action = [vec_env.action_space.sample()]
  obs, _, _, _ = vec_env.step(action)
# Save the video
vec_env.close()

and I get this error

python test.py
Traceback (most recent call last):
  File "/home/intelligence-lab-pc4/Documents/kalli/test_mjc3/test.py", line 17, in <module>
    vec_env.reset()
  File "/home/intelligence-lab-pc4/Documents/kalli/test_mjc3/stable-baselines3/stable_baselines3/common/vec_env/vec_video_recorder.py", line 66, in reset
    self.start_video_recorder()
  File "/home/intelligence-lab-pc4/Documents/kalli/test_mjc3/stable-baselines3/stable_baselines3/common/vec_env/vec_video_recorder.py", line 78, in start_video_recorder
    self.video_recorder.capture_frame()
    ^^^^^^^^^^^^^^^^^^^
  File "/home/intelligence-lab-pc4/Documents/kalli/test_mjc3/stable-baselines3/stable_baselines3/common/vec_env/base_vec_env.py", line 420, in __getattr__
    return self.getattr_recursive(name)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/intelligence-lab-pc4/Documents/kalli/test_mjc3/stable-baselines3/stable_baselines3/common/vec_env/base_vec_env.py", line 445, in getattr_recursive
    attr = getattr(self.venv, name)
           ^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'DummyVecEnv' object has no attribute 'video_recorder'

i am using gymnasium==1.0.0a1, sb3 this PR's latest commit (in march)

@pseudo-rnd-thoughts
Copy link
Contributor Author

Just updating VecRecordEnv needs to be fully updated / rewritten

@Kallinteris-Andreas Yes, this is the only part of the PR that still needs to be done.
I'm tempted to just copy and paste the old video recorder in as a solution. Might try to do this evening

@qgallouedec
Copy link
Collaborator

Feel free to ping me if necessary

tests/test_logger.py Outdated Show resolved Hide resolved
@pseudo-rnd-thoughts
Copy link
Contributor Author

If I understand correctly, SB3 has its only vector environments, VecEnv so don't use the gymnasium vector environment so is unaffected by this change

@araffin
Copy link
Member

araffin commented Oct 9, 2024

is there anything blocking a new SB3 release

There are different things blocking:

  • time and priority from my side
  • testing with SB3 contrib and RL Zoo
  • backward compat and tests with 0.28.1
  • if possible, remove the copy-pasted VideoRecorder, or at least bring it to SB3 standards in term of documentation/typing/tests

@Kallinteris-Andreas
Copy link
Contributor

  1. I will (try to test) backcompat for gymnasium==0.29.1
  2. Is there a reason you need the VideoRecorder to be inside SB3, can't you just use the one directly from gymnasium
  3. unfortunately my time machine no longer works, so I can not give you more time

@araffin
Copy link
Member

araffin commented Oct 12, 2024

Is there a reason you need the VideoRecorder to be inside SB3, can't you just use the one directly from gymnasium

we don't: #1837 (comment)

@pseudo-rnd-thoughts
Copy link
Contributor Author

Is there a reason you need the VideoRecorder to be inside SB3, can't you just use the one directly from gymnasium

SB3 doesn't use Gymnasium's VectorEnv and has their own VecEnv and so have their own wrappers like VecVideoRecorder. This is largely a wrapper about the Gymnasium old monitor code however as that was removed in v1.0, then either SB3 needs the original code, a stripped down version or a complete rewrite of the wrapper.

@araffin araffin added the help wanted Help from contributors is welcomed label Oct 29, 2024
@araffin
Copy link
Member

araffin commented Nov 2, 2024

I've opened two other PR:

and integrated the video recorder inside the VecVideoRecorder.

unit tests are passing except for trained agents in the RL Zoo (new version of envs).

What is missing for this PR is to update the changelog.

@Kallinteris-Andreas
Copy link
Contributor

Are the unit test failing for the same version of the environments?

@araffin
Copy link
Member

araffin commented Nov 4, 2024

Are the unit test failing for the same version of the environments?

it's just that the env LunarLander-v2 doesn't exist anymore.

Copy link
Collaborator

@qgallouedec qgallouedec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good :)

@araffin araffin merged commit 8f0b488 into DLR-RM:master Nov 4, 2024
4 checks passed
@araffin
Copy link
Member

araffin commented Nov 4, 2024

I've commented out the lunar lander envs in DLR-RM/rl-baselines3-zoo#475 (i need to update the trained agents), but there is still one failure: parking-v0 obs space from highway-env was updated (float64 instead of float32) but the version number is the same.

@araffin
Copy link
Member

araffin commented Nov 5, 2024

@pseudo-rnd-thoughts there is another weird bug that just popped with gymnasium v1: https://github.com/DLR-RM/rl-baselines3-zoo/actions/runs/11664748980/job/32475927244?pr=475

The saved inf is no longer the same?

@pseudo-rnd-thoughts
Copy link
Contributor Author

pseudo-rnd-thoughts commented Nov 5, 2024

@araffin Thanks for reporting that. I'm reading the logs, and I'm a bit confused by what is happening.
You are loading a CartPole-v1 model and finding that the observation space is different?
It seems to be related to Farama-Foundation/Gymnasium#1092
There was a time when we avoided inf bounds due to the environment checker, but I think this reverted as we realised that the max int value causes more issues than inf.

Apologies for not noting this in the release notes. I'm not sure what we do about it now, any thoughts?

@araffin
Copy link
Member

araffin commented Nov 5, 2024

You are loading a CartPole-v1 model and finding that the observation space is different?

yes, from

# Check if given env is valid
check_for_correct_spaces(env, data["observation_space"], data["action_space"])
some good sanity checks.

I see, then I need to update the trained agent (save and load it while overriding the obs space).

@araffin
Copy link
Member

araffin commented Nov 5, 2024

Should fixed in DLR-RM/rl-trained-agents@eb1bd43 but in the meantime I found another issue.
Is there a way to resize the render window when using ale-py? (the render on my machine is way too big and the window cannot be adjusted, a quick fix is to use SubprocVecEnv and opencv display but that's not so nice...)

@pseudo-rnd-thoughts
Copy link
Contributor Author

Glad the other issue is solved.

For ale-py, I'm surprised this is a new problem. Another user has recently asked for changes to the render size. I will investigate adding an argument for specifying the render window size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Help from contributors is welcomed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] When are you planning to upgrade to Gymnasium v1.0.0
5 participants