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

Revert "Revert "Revert "Revert "[RLlib] Upgrade to gymnasium 1.0.0 (ale_py 0.10.1, mujoco 3.2.4, pettingzoo 1.24.3 supersuit 3.9.3)."""" #48443

Conversation

sven1977
Copy link
Contributor

Reverts #48435

@aslonnie
Copy link
Collaborator

will leave this to @can-anyscale and other people. let me know if you need explicit approval on the setup.py file.

we might be breaking a record of the number of "revert"'s in the title..

maybe consider breaking this PR into smaller ones so that it is easier to make progress(though I am not sure what is the best way to break it)? maybe we can accept using gymnasium ~= 1.0 in setup.py to have some wiggle room?

Copy link
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

all power to @can-anyscale

…rt-48435-revert-48308-revert-48297-revert-45328-upgrade_gymnasium_to_1_0_0a1
Signed-off-by: sven1977 <svenmika1977@gmail.com>
Copy link
Collaborator

@simonsays1980 simonsays1980 left a comment

Choose a reason for hiding this comment

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

LGTM. Huge change now. Maybe I overread it, but could you change the environment also in the BC benchmark with RLUnplugged?

@@ -259,7 +263,7 @@ def _sample_timesteps(
to_env = {
Columns.ACTIONS: [
{
aid: self.env.get_action_space(aid).sample()
aid: self.env.unwrapped.get_action_space(aid).sample()
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this is now the new thing: always unwrap?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only RLlib-internally b/c the vector env is wrapped to return info-lists (rather than dicts).
I think we still have to unify the way the user sees these envs in their callbacks: Always as-is (vector wrapper) or always as unwrapped (pure gym.vector.Env). Both approaches have advantages and disadvantages.


ts += self.num_envs
observations, rewards, terminateds, truncateds, infos = results
observations, actions = unbatch(observations), unbatch(actions)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't there a gymnasium wrapper that does this?

"rllib-single-agent-env-v0",
num_envs=self.config.num_envs_per_env_runner,
asynchronous=self.config.remote_worker_envs,
vectorization_mode=(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice! I remember in my problems with the update nto gymnasium==1.0.0 this was one reason of failure. It needs now this mode.

…rt-48435-revert-48308-revert-48297-revert-45328-upgrade_gymnasium_to_1_0_0a1
Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 enabled auto-merge (squash) November 1, 2024 08:29
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Nov 1, 2024
@sven1977
Copy link
Contributor Author

sven1977 commented Nov 1, 2024

Hey @aslonnie , could we give this a go? Release tests are passing properly now:
https://buildkite.com/ray-project/release/builds/25039

@sven1977 sven1977 added rllib RLlib related issues rllib-env rllib env related issues rllib-newstack rllib-envrunners Issues around the sampling backend of RLlib tests-ok The tagger certifies test failures are unrelated and assumes personal liability. labels Nov 1, 2024
@sven1977 sven1977 merged commit 99a9a7b into master Nov 1, 2024
7 checks passed
@sven1977 sven1977 deleted the revert-48435-revert-48308-revert-48297-revert-45328-upgrade_gymnasium_to_1_0_0a1 branch November 1, 2024 17:14
Jay-ju pushed a commit to Jay-ju/ray that referenced this pull request Nov 5, 2024
…le_py 0.10.1, mujoco 3.2.4, pettingzoo 1.24.3 supersuit 3.9.3)."""" (ray-project#48443)
JP-sDEV pushed a commit to JP-sDEV/ray that referenced this pull request Nov 14, 2024
…le_py 0.10.1, mujoco 3.2.4, pettingzoo 1.24.3 supersuit 3.9.3)."""" (ray-project#48443)

Signed-off-by: JP-sDEV <jon.pablo80@gmail.com>
mohitjain2504 pushed a commit to mohitjain2504/ray that referenced this pull request Nov 15, 2024
…le_py 0.10.1, mujoco 3.2.4, pettingzoo 1.24.3 supersuit 3.9.3)."""" (ray-project#48443)

Signed-off-by: mohitjain2504 <mohit.jain@dream11.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go add ONLY when ready to merge, run all tests rllib RLlib related issues rllib-env rllib env related issues rllib-envrunners Issues around the sampling backend of RLlib rllib-newstack tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants