cogment / cogment-verse

Research platform for Human-in-the-loop learning (HILL) & Multi-Agent Reinforcement Learning (MARL)
https://cogment.ai/cogment_verse
Apache License 2.0
78 stars 15 forks source link

Update PettingZoo version, make stylization/spelling consistent #176

Closed elliottower closed 1 year ago

elliottower commented 1 year ago

This PR closes https://github.com/cogment/cogment-verse/issues/142

I've run the tests locally both before and after doing this change and it passed, so I think it shouldn't be an issue.

Overcooked AI is in the process of updating their PettingZoo to the latest version (https://github.com/HumanCompatibleAI/overcooked_ai/pull/123), which I've been helping them on, so that shouldn't be an issue either.

Only issue I've encountered is after bumping the versions, reinstalling via requirements.txt I get this error for most of the packages (using pip in a conda virtual env): INFO: This is taking longer than usual. You might need to provide the dependency resolver with stricter constraints to reduce runtime. See https://pip.pypa.io/warnings/backtracking for guidance. If you want to abort this run, press Ctrl + C.

elliottower commented 1 year ago

Is it intentional that SuperSuit is not in the requirements.txt? I see some of the documentation says you have to manually install it, is it that it's a problem for certain envs? I had to manually install it (the newest version 3.8.0 works as well), could I add that to the requirements?

Also, not sure if this is a future planned feature but it's probably best to move the requirements into the pyproject.toml and have optional requirements, so for example opting in to do pip install cogment-verse[pettingzoo] or pip install cogment-verse[isaac-gym] or pip install cogment-verse[all, testing], etc. I could help with that if need be, as we do it for most of our Farama projects

elliottower commented 1 year ago

Thanks for the changes! I had actually started to fix this spelling error in a current branch I am working on.

Oh I see, okay. I just figured I could do it real quick and check that the PettingZoo/SuperSuit/Gymnasium versions were all compatible and fix any issues related to them, as I am a maintainer of those projects, but it appears there are no issues. The only major change for PettingZoo is that with parallel environments, the reset() call now does not have the return_info argument, and always returns both the observation and info, but I see no occurrences of that in the codebase.

wduguay-air commented 1 year ago

The SuperSuit version installed outside of the requirements was a similar issue we experience when support for gym. They had conflicting pygame version requirements, but it still worked fine if we installed it separately. Really not an ideal solution. I am currently looking at options to remove this unnecessary step as well as bumping the PettingZoo version.

elliottower commented 1 year ago

The SuperSuit version installed outside of the requirements was a similar issue we experience when support for gym. They had conflicting pygame version requirements, but it still worked fine if we installed it separately. Really not an ideal solution. I am currently looking at options to remove this unnecessary step as well as bumping the PettingZoo version.

I'm not sure why but somehow I was able to install the newest versions without any issues, though I can see that there are indeed different pygame versions required. I'd say the optional requirements in pyproject or setup.py is probably the best way to get around this issue (see the other issue linked above, I explain a bit more). The current versions I have which seem to work together are pettingzoo==1.23.1, gymnasium==0.28.1, gym==0.25.2, pygame==2.1.2

Edit: ahhh I see the reason, it was that I installed base PettingZoo which doesn't have any pygame requirements, but any environments such as pettingzoo[classic] do require pygame 2.3.0. Not sure how your CI works but I hope it catches things like this.

elliottower commented 1 year ago

Just tested some more and yeah as you say, installing separately means that it updates the pygame version, but installing all in the same requirements.txt has the conflicting pygame versions. So the only solution then would indeed be to do optional requirements and have separate installations for gym and for pettingzoo for example

wduguay-air commented 1 year ago

Do you think this could be fixed if we installed gym as you mentionned?

But a solution to this is done in our Shimmy and ViZDoom projects, which support both Gymnasium and Gym, using optimal requirements (pip install shimmy[gym-v21])

If not, then I'll find a solution to have optional requirements on our side. I will actually need to bump because I am working on an integration for the MPE environments and I need the fix for black screen rendering.

wduguay-air commented 1 year ago

Hi @elliottower, there have been a ton of changes in Cogment-Verse since this PR. I incorporated your provided naming accuracy and consistency of "PettingZoo" spelling into this PR: https://github.com/cogment/cogment-verse/pull/187, which also bumps the PettingZoo and Gymnasium versions.

If you agree, I would close this PR.

elliottower commented 1 year ago

Hi @elliottower, there have been a ton of changes in Cogment-Verse since this PR. I incorporated your provided naming accuracy and consistency of "PettingZoo" spelling into this PR: #187, which also bumps the PettingZoo and Gymnasium versions.

If you agree, I would close this PR.

Sounds good, thanks!

elliottower commented 1 year ago

Hi @elliottower, there have been a ton of changes in Cogment-Verse since this PR. I incorporated your provided naming accuracy and consistency of "PettingZoo" spelling into this PR: #187, which also bumps the PettingZoo and Gymnasium versions. If you agree, I would close this PR.

Sounds good, thanks!

Oh one note is I’m going to be making a PettingZoo release in the next few days to fix compatibility issues with the new Gymnasium v29, so it might be worth waiting a bit until that’s out and then you won’t have to update again for a while (major update we’ve been working on and happens to coincide with gymnasium)

wduguay-air commented 1 year ago

Okay! Thanks for the heads up. However, I did not experience any issues with Gymnasium v0.29 and PettingZoo v1.23.1