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

[RLlib] Update PettingZoo wrapper to current API (1.23.0) #34696

Closed
wants to merge 14 commits into from

Conversation

elliottower
Copy link

@elliottower elliottower commented Apr 22, 2023

Using a patch rather than a branch just for simplicity. Updated requirements:

  • gymnasium version (0.28.1, only fixes bugs no major API changes)

  • supersuit version (3.8.0, from 3.7.0, removes return_info argument on reset)

  • pettingzoo version (1.23.0 from 1.22.1, removes return_info argument on reset)

  • Removed gym from requirements, as Gymnasium supports atari games (code comment below)
    # TODO(sven): Still needed for Atari (need to be wrapped by gymnasium as it does NOT support Atari yet)

  • Removed chess from requirements, as PettingZoo installs it with the classic optional

# When installing pettingzoo, chess is missing, even though its a dependancy
# TODO: remove if a future pettingzoo and/or ray version fixes this dependancy issue
chess==1.7.0

Other changes:

  • Removed the render_mode calls in pettingzoo_env is because render() does not take an argument anymore.
  • Removed return_info from reset calls in pettingzoo_env reset() calls, as it does not take that argument anymore.

From the previous PR:

Render() also has not taken the render_mode argument since October of 2022: Farama-Foundation/PettingZoo@a74a933

Note: The newest PettingZoo release (1.23.0) has removed return_info and fixes a typo with BaseParallelWraper, SuperSuit 3.8.0 mirrors these changes.

Most of these changes were approved in this previous PR, but I had to close it because of issues with incorrectly signed off commits and git merge conflicts (sorry about that, always forget that you need to both sign and sign off on commits with -s)

Why are these changes needed?

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

also removes rendering mode from `render()` as that is specified in the environment initialization rather than in `render()`

Signed-off-by: Elliot Tower <[email protected]>
@elliottower
Copy link
Author

Upgrading gymnasium to 0.28.1 causes this test to fail due to unknown version, but it lists the version in the options so I'm not sure what the problem is. I tried running the exact same command locally and was able to install (pip install gymnasium[atari,mujoco]==0.28.1)

#10 69.05 ERROR: Could not find a version that satisfies the requirement gymnasium[atari,mujoco]==0.28.1 (from versions: 0.0.1, 0.26.1, 0.26.2, 0.26.3, 0.27.0, 0.27.1, 0.28.0, 0.28.1)
--
#10 ERROR: executor failed running [/bin/sh -c RLLIB_TESTING=1 TRAIN_TESTING=1 TUNE_TESTING=1 bash --login -i 

https://buildkite.com/ray-project/oss-ci-build-pr/builds/19560#0187ab50-b26f-4f01-a8b2-f0783a178337

@elliottower
Copy link
Author

@sven1977 @avnishn Could one of you guys help get these changes done? If it's easier for someone to just make a separate PR I don't mind, it just really needs to be done. I'll see if I can fix the build failures myself but as I posted above there are some issues I couldn't figure out.

The Gymnasium hard requirement of 0.26.3 prevents users from installing ray[rllib] with any more recent versions of PettingZoo/Gymnasium. We are finally (sorry for the delays) releasing the next major PettingZoo release along with a SuperSuit release, so I will update this tomorrow with that.

@elliottower
Copy link
Author

Found the issue with installation, was a conflict with the base python requirements/setup.py fixing gymnasium==0.26.2, when I replaced all occurrences with 0.28.1 it's able to install, but now there seem to be some slight performance differences (e.g., DQN_CartPole_v1 reaching 65 but not 70 avg reward: link, talked with Mark who maintains Gymnasium and he says the environment has not changed since then, but other changes since then may be causing the different performance.

@elliottower
Copy link
Author

Looked through all the CI errors and there doesn't seem to be anything very helpful. Tried to run locally and had some issues with installing with bazel, would appreciate it if someone else could help me narrow down what the problems actually are. @sven1977

@elliottower elliottower changed the title [RLlib] Update gymnasium/supersuit/pettingzoo requirements [RLlib] Update PettingZoo wrapper to current API (1.23.0) May 23, 2023
@elliottower
Copy link
Author

Waiting on #35698 to fix issues with gymnasium, but any help figuring out the pettingzoo-specific build fails (like training accuracy being slightly different) would be much appreciated

@agoeckner
Copy link

If I need to use Pettingzoo and rllib together right now, which versions should I install?

@avnishn avnishn self-assigned this May 25, 2023
@elliottower
Copy link
Author

If I need to use Pettingzoo and rllib together right now, which versions should I install?

PettingZoo 1.22.3 works with current versions of Ray, and the current tutorials on PettingZoo's website (pistonball, simple poker) should work with that version. This PR allows the new 1.23.0 release to work with RLlib, but hasn't been extensively tested yet so I can't guarantee things on ray's side will work 100% as intended (you're probably best off using 1.22.3 in the meantime)

@gresavage
Copy link

gresavage commented May 25, 2023

Found the issue with installation, was a conflict with the base python requirements/setup.py fixing gymnasium==0.26.2, when I replaced all occurrences with 0.28.1 it's able to install, but now there seem to be some slight performance differences (e.g., DQN_CartPole_v1 reaching 65 but not 70 avg reward: link, talked with Mark who maintains Gymnasium and he says the environment has not changed since then, but other changes since then may be causing the different performance.

I am noticing a similar drop in performance between subclassing from MultiAgentEnv vs subclassing from ParallelEnv and wrapping with a slightly modified ParallelPettingZooEnv wrapper which addresses #32889

For context, both runs were using RLlibs PPO algorithm on LunarLanderContinuous-v1 with a single agent:
gymnasium==0.28.1
numpy==1.22.2
pygame==2.1.0
ray==2.3.0
pettingzoo==1.23.0
torch==1.13.0

@elliottower I assume you have confirmed this already, but are algorithms in PettingZoo performing the same between 1.22+ and 1.23+? What did Mark indicate were potential causes?

@elliottower
Copy link
Author

Found the issue with installation, was a conflict with the base python requirements/setup.py fixing gymnasium==0.26.2, when I replaced all occurrences with 0.28.1 it's able to install, but now there seem to be some slight performance differences (e.g., DQN_CartPole_v1 reaching 65 but not 70 avg reward: link, talked with Mark who maintains Gymnasium and he says the environment has not changed since then, but other changes since then may be causing the different performance.

I am noticing a similar drop in performance between subclassing from MultiAgentEnv vs subclassing from ParallelEnv and wrapping with a slightly modified ParallelPettingZooEnv wrapper which addresses #32889

For context, both runs were using RLlibs PPO algorithm on LunarLanderContinuous-v1 with a single agent: gymnasium==0.28.1 numpy==1.22.2 pygame==2.1.0 ray==2.3.0 pettingzoo==1.23.0 torch==1.13.0

@elliottower I assume you have confirmed this already, but are algorithms in PettingZoo performing the same between 1.22+ and 1.23+? What did Mark indicate were potential causes?

The only major change is the fact that reset now returns info by default, but that shouldn’t affect training. Haven’t had a chance to test much unfortunately, but would be curious if you get a chance to look into it more.

@elliottower
Copy link
Author

Found the issue with installation, was a conflict with the base python requirements/setup.py fixing gymnasium==0.26.2, when I replaced all occurrences with 0.28.1 it's able to install, but now there seem to be some slight performance differences (e.g., DQN_CartPole_v1 reaching 65 but not 70 avg reward: link, talked with Mark who maintains Gymnasium and he says the environment has not changed since then, but other changes since then may be causing the different performance.

I am noticing a similar drop in performance between subclassing from MultiAgentEnv vs subclassing from ParallelEnv and wrapping with a slightly modified ParallelPettingZooEnv wrapper which addresses #32889

For context, both runs were using RLlibs PPO algorithm on LunarLanderContinuous-v1 with a single agent: gymnasium==0.28.1 numpy==1.22.2 pygame==2.1.0 ray==2.3.0 pettingzoo==1.23.0 torch==1.13.0

@elliottower I assume you have confirmed this already, but are algorithms in PettingZoo performing the same between 1.22+ and 1.23+? What did Mark indicate were potential causes?

Could you give more details about how exactly you changed the wrapper and got different performance? Trying to figure out what the problem would be

@gresavage
Copy link

gresavage commented Jul 12, 2023 via email

# Conflicts:
#	python/requirements.txt
#	python/requirements/ml/requirements_rllib.txt
#	rllib/env/wrappers/pettingzoo_env.py
@elliottower
Copy link
Author

Merging upstream remote into this branch seems to have created some issues, so I'm going to just close this and do a new PR. Looks like PettingZoo 1.23.1 is now supported with master, so that's good to see.

@elliottower elliottower closed this Aug 7, 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