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 to gym 0.26 #12

Closed
wants to merge 1 commit into from
Closed

Conversation

timokau
Copy link
Contributor

@timokau timokau commented Nov 15, 2022

Note: I realize that this is a big change and you may decide not to merge this. I worked on it for my own purposes (applying the avalon dreamer baseline to gym 0.26 environments) and thought it would be best to propose it upstream, but I understand if you decide to close the PR. I have also not tested this exhaustively, I have only verified that it works for my purposes. So it is likely that there are still some mistakes and this PR would be a starting point rather than an immediately mergeable.

If you want me to run the test suite, I'd be happy to. In that case please let me know how best to do that (use the dockerfile? pytest?).


See [1] for a list of breaking changes. In particular we now

  • need to pass "render_mode" when initializing a gym environment, especially
    when we want to PixelObservationWrapper later,

  • need to split done into truncated and terminated and

  • need to return info in reset.

Note that future Gym development will happen in the "Gymnasium" [2] fork, which
is based on Gym 0.26. Migrating from Gym 0.26 to Gymnasium should be easy.

[1] https://github.com/openai/gym/releases/tag/0.26.0
[2] https://github.com/Farama-Foundation/Gymnasium

See [1] for a list of breaking changes. In particular we now

- need to pass "render_mode" when initializing a gym environment, especially
  when we want to PixelObservationWrapper later,

- need to split `done` into `truncated` and `terminated` and

- need to return `info` in `reset`.

Note that future Gym development will happen in the "Gymnasium" [2] fork, which
is based on Gym 0.26. Migrating from Gym 0.26 to Gymnasium should be easy.

[1] https://github.com/openai/gym/releases/tag/0.26.0
[2] https://github.com/Farama-Foundation/Gymnasium
@mx781
Copy link
Collaborator

mx781 commented Dec 6, 2022

@timokau appreciate the PR, thanks! Just to signal intention - this is indeed an upgrade we're looking to do, but we were waiting for the new API to stabilize (perhaps when v.1.0 is released?). Part of the reason is that we run mypy against our entire codebase and would prefer the types to fully settle before making this upgrade.

For now we'll leave this PR open, but you've done a fair bit of legwork here so it should come handy once we upgrade - we'll make sure to attribute appropriately in any commits we pull in - thanks!

@timokau
Copy link
Contributor Author

timokau commented Dec 6, 2022

Great, thanks for the update :) For what it's worth, I think gym(nasium) 0.26 is supposed to be fairly stable. Quoting from the release notes:

All of the previously "turned off" changes of the base API (step termination / truncation, reset info, no seed function, render mode determined by initialization) are now expected by default. We still plan to make breaking changes to Gym itself, but to things that are very easy to upgrade (environments and wrappers), and things that aren't super commonly used (the vector API). Once those aspects are stabilized, we'll do a proper 1.0 release and follow semantic versioning. Additionally, unless something goes terribly wrong with this release and we have to release a patched version, this will be the last release of Gym for a while.

But yes, the dust has probably settled a bit more by the time 1.0 is released.

@micimize micimize removed their request for review December 6, 2022 18:00
@micimize
Copy link
Collaborator

micimize commented Dec 6, 2022

Yeah – we'll see. Realistically, there's a decent amount on their roadmap that could end up impacting that plan once they're in the weeds. But if they stay on schedule we should know relatively soon 🙂

Another consideration for me is that I'd like to put off unpacking done into terminated, truncated until we have the bandwidth to do it rigorously so that the semantic distinction is actually honored, which will entail adding some logic on the godot side and to the bridge. Not unmanageable but will require some attention.

Did you try using EnvCompatibility & were there issues? If not, maybe loosening our dependency constraint and documenting the usage would be a good stop-gap

@timokau
Copy link
Contributor Author

timokau commented Dec 23, 2022

Another consideration for me is that I'd like to put off unpacking done into terminated, truncated until we have the bandwidth to do it rigorously so that the semantic distinction is actually honored, which will entail adding some logic on the godot side and to the bridge. Not unmanageable but will require some attention.

That makes sense, getting this entirely right probably requires digging a bit deeper.

Did you try using EnvCompatibility & were there issues? If not, maybe loosening our dependency constraint and documenting the usage would be a good stop-gap

No I didn't try that, I don't remember why. Probably should have tried that first.

@zplizzi
Copy link
Collaborator

zplizzi commented Jan 27, 2023

I'm going to go ahead and close this, given that we'll probably start fresh when we're ready to make this change. Thanks for proposing it though!

@zplizzi zplizzi closed this Jan 27, 2023
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.

4 participants