-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Gymnasium Integration #789
Conversation
… to only use new step API
… API) and fixed FiniteVectorEnv
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #789 +/- ##
==========================================
+ Coverage 90.89% 91.10% +0.20%
==========================================
Files 73 73
Lines 5109 5079 -30
==========================================
- Hits 4644 4627 -17
+ Misses 465 452 -13
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hey @Markus28, I'm actually thinking about a non-breaking change. If a user uses gym env, tianshou's VectorEnv itself will first detect the class of the env, and if it's gym env then we adapt shimmy wrapper on top of that. This is an automated process and won't degrade user-experience. The current changes looks fine for me but it would be nice to add the above feature request! Also, have you added shimmy usage in docs or readme so that people can know how to convert it to a gymnasium env without seeking other resources for help? |
I can do that :) |
So it turns out that there is unfortunately no "universal" shimmy wrapper that will take any Gym environment and give us an up-to-date Gymnasium environment. Shimmy implements different wrappers for the different API versions. I think we have the following options:
Mark seems to believe that doing a universal wrapper would be quite complicated, but I don't see why it shouldn't be doable. I'm not sure whether it would then be part of Shimmy or Tianshou though. |
I personally in favor of the second option. This will reduce most of (90%) the issue user hit and will reduce the maintenance effort quite a lot. |
gym_version = packaging.version.parse(old_gym.__version__) | ||
if gym_version >= packaging.version.parse("0.26.0"): | ||
return shimmy.GymV26CompatibilityV0(env=env) | ||
elif gym_version >= packaging.version.parse("0.22.0"): | ||
return shimmy.GymV22CompatibilityV0(env=env) | ||
else: | ||
raise Exception( | ||
f"Found OpenAI Gym version {gym.__version__}. " | ||
f"Tianshou only supports OpenAI Gym environments of version>=0.22.0" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pseudo-rnd-thoughts Can you confirm that this is how the shimmy wrappers are intended to be used? I.e. for any environment 0.26.0>gym.__version__>=0.22.0
we use GymV22CompatibilityV0"
and for gym.__version__ >= 0.26.0
we use GymV26CompatibilityV0
?
I implemented the second version. I will still need to update some documentation and the tutorials :) There are currently no tests for OpenAI Gym environments, let me know if I should add some. |
Yes, I think we definitely need a test which imports |
@Trinkle23897 This PR is failing the doc spelling check but the error message is quite cryptic:
I have no idea how to fix it. Can you take a look? Other than that, I think this PR is ready to merge. |
will take a look tonight |
hmm on my local:
|
You need to update your gym version, but we will update shimmy to avoid this issue |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leave render_collection
as-is, will educate user to upgrade the gym version as temporary solution
Changes: - Disclaimer in README - Replaced all occurences of Gym with Gymnasium - Removed code that is now dead since we no longer need to support the old step API - Updated type hints to only allow new step API - Increased required version of envpool to support Gymnasium - Increased required version of PettingZoo to support Gymnasium - Updated `PettingZooEnv` to only use the new step API, removed hack to also support old API - I had to add some `# type: ignore` comments, due to new type hinting in Gymnasium. I'm not that familiar with type hinting but I believe that the issue is on the Gymnasium side and we are looking into it. - Had to update `MyTestEnv` to support `options` kwarg - Skip NNI tests because they still use OpenAI Gym - Also allow `PettingZooEnv` in vector environment - Updated doc page about ReplayBuffer to also talk about terminated and truncated flags. Still need to do: - Update the Jupyter notebooks in docs - Check the entire code base for more dead code (from compatibility stuff) - Check the reset functions of all environments/wrappers in code base to make sure they use the `options` kwarg - Someone might want to check test_env_finite.py - Is it okay to allow `PettingZooEnv` in vector environments? Might need to update docs?
Changes:
PettingZooEnv
to only use the new step API, removed hack to also support old API# type: ignore
comments, due to new type hinting in Gymnasium. I'm not that familiar with type hinting but I believe that the issue is on the Gymnasium side and we are looking into it.MyTestEnv
to supportoptions
kwargPettingZooEnv
in vector environmentStill need to do:
options
kwargPettingZooEnv
in vector environments? Might need to update docs?