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] Issue 39453: PettingZoo wrappers should use correct multi-agent dict action- and observation spaces. #39459

Conversation

sven1977
Copy link
Contributor

@sven1977 sven1977 commented Sep 8, 2023

See issue: #39453

Solution:
Change both PettingZoo adapters of RLlib to abide to RLlib's multi-agent action- and observation space convention (gym.spaces.Dict mapping agent Ids to individual spaces).

Closes: #39453 and #32889

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 :(

Signed-off-by: sven1977 <svenmika1977@gmail.com>
@chrisyeh96
Copy link

chrisyeh96 commented Sep 8, 2023

Thanks @sven1977 for responding so quickly to #39453!

I have a suggestion: can we make RLLib's PettingZooEnv and ParallelPettingZooEnv wrappers support PettingZoo environments where agents have different action_spaces and observation_spaces? This would actually simplify the RLLib wrapper code quite significantly.

For example, here's what the __init__() method for ParallelPettingZooEnv would look like:

class ParallelPettingZooEnv(MultiAgentEnv):
    def __init__(self, env):
        super().__init__()
        self.par_env = env
        self.par_env.reset()
        self._agent_ids = set(self.par_env.agents)

        self.observation_space = gym.spaces.Dict(self.par_env.observation_spaces)
        self.action_space = gym.spaces.Dict(self.par_env.action_spaces)

…e_39453_pettingzoo_wrappers_should_use_correct_ma_dict_spaces
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977
Copy link
Contributor Author

Hey @chrisyeh96 , thanks for the hint. Changed this to be even more flexible now for both parallel and non-parallel env wrappers.

first_obs_space
)
self.action_space = convert_old_gym_space_to_gymnasium_space(first_action_space)

self._agent_ids = self.env.agents

Choose a reason for hiding this comment

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

self._agent_ids should be a set, so this line should be changed to:

self._agent_ids = set(self.env.agents)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -188,37 +154,10 @@ def __init__(self, env):
super().__init__()
self.par_env = env
self.par_env.reset()
self._agent_ids = self.par_env.agents

Choose a reason for hiding this comment

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

self._agent_ids should be a set, so this line should be changed to:

self._agent_ids = set(self.par_env.agents)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@chrisyeh96
Copy link

Thanks @sven1977 for the changes! I've suggested 2 minor changes in my code review.

There's also one other place that should be updated. The docstring for PettingZooEnv currently still says that the wrapper only supports environments with homogeneous action / observation spaces. With this pull request, this comment should be removed. See below:

1. All agents have the same action_spaces and observation_spaces.
Note: If, within your aec game, agents do not have homogeneous action /
observation spaces, apply SuperSuit wrappers
to apply padding functionality: https://github.com/Farama-Foundation/
SuperSuit#built-in-multi-agent-only-functions

Thanks for your work on this!

Signed-off-by: sven1977 <svenmika1977@gmail.com>
Signed-off-by: sven1977 <svenmika1977@gmail.com>
@sven1977 sven1977 added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Sep 12, 2023
@chrisyeh96
Copy link

Thanks @sven1977! Looking forward to seeing this change merged. Do you know if it will be included in the next release of RLLib?

@sven1977
Copy link
Contributor Author

Hey @chrisyeh96 , yes, it'll be part of Ray 2.8, though, and won't make it into the upcoming 2.7 anymore.

@chrisyeh96
Copy link

Understood. Any idea on what the timeline for Ray 2.8 will be? For me, this issue is quite important.

I have a project called SustainGym where I've made 5 RL environments related to energy / sustainability applications. 3 of the 5 environments are multi-agent RL environments following the PettingZoo API.

Currently, I include RLLib as a dependency of SustainGym, because I have special versions of my environments to fit the RLLib MultiAgentEnv API. In theory, SustainGym should not depend on RLLib, because a RL environment package should be agnostic to the RL algorithm. Once this pull request gets merged, I can remove the RLLib dependency and instead direct SustainGym users to simply use the RLLib wrapper around the SustainGym PettingZoo env if they want to use RLLib algorithms.

@sven1977
Copy link
Contributor Author

2.8 will be released a few weeks after the Ray Summit (which is next week). So roughly beginning to mid October.

@sven1977 sven1977 merged commit 1fbb143 into ray-project:master Sep 21, 2023
38 of 41 checks passed
simonsays1980 pushed a commit to simonsays1980/ray that referenced this pull request Sep 26, 2023
…nt dict action- and observation spaces. (ray-project#39459)

Signed-off-by: Simon Zehnder <simon.zehnder@gmail.com>
vymao pushed a commit to vymao/ray that referenced this pull request Oct 11, 2023
…nt dict action- and observation spaces. (ray-project#39459)

Signed-off-by: Victor <vctr.y.m@example.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

[RLLib] ParallelPettingZooEnv wrapper fails rllib.utils.check_env()
3 participants