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

Integrating Gymnasium #222

Merged
merged 19 commits into from
Dec 9, 2022
Merged

Conversation

Markus28
Copy link
Collaborator

@Markus28 Markus28 commented Nov 18, 2022

This PR leaves gym as it is and adds gymnasium environments in an analogous way.

@Markus28
Copy link
Collaborator Author

This implementation leaves the Gym envpool part entirely unmodified. My changes add a Gymnasium option, which mostly mirrors the Gym option. Specs now have the additional attributes gymnasium_action_space and gymnasium_observation_space. The attributes action_space and observation_space remain reserved for Gym, preserving backward compatibility.
I will still have to add some tests and play around with it locally to make sure that everything works as expected. Also, the documentation needs to be updated accordingly.

Were these roughly the changes we had in mind @Trinkle23897? I hope I understood you correctly on discord :).

@Trinkle23897
Copy link
Collaborator

That makes sense! I’ll take a look these days. Great job and huge thanks!

@Trinkle23897 Trinkle23897 marked this pull request as ready for review December 4, 2022 21:05
CarRacingEnvSpec, CarRacingDMEnvPool, CarRacingGymEnvPool = py_env(
_CarRacingEnvSpec, _CarRacingEnvPool
)
(
Copy link
Member

@mavenlin mavenlin Dec 9, 2022

Choose a reason for hiding this comment

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

@Trinkle23897 Would it be better that we can make this less verbose? e.g. with a helper function like

def register_to_py(cpp_shared_lib):
  dict_of_name_to_env = py_env(cpp_shared_lib)
  globals().update(dict_of_name_to_env)
  __all__ = dict_of_name_to_env.keys()

Copy link
Member

Choose a reason for hiding this comment

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

I mean, we could make this change as a separate PR after this one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good to me

@Trinkle23897 Trinkle23897 merged commit 494a409 into sail-sg:main Dec 9, 2022
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.

3 participants