Gym fixes - Follow up from #705 #734
Conversation
araffin
added
the
Maintainers on vacation
|
I think closing this was a side-effect of deleting the branch, I'm going to reopen and try to rebase |
|
👀 ok no more git-gymnastics for me at 3am. I have no idea how this happened... |
I think this is a GitHub bug! What I saw earlier was that you deleted |
Hey @AdamGleave thanks for reviewing, I'm a bit swamped right now and won't have time to look at this until probably late next week or beginning of the week after. So in the interest of getting it merged, it would be great if you find someone in the mean time to finish it up! |
|
Merging now. @araffin I've opened PRs in SB example repos & zoo. No rush, can take a look at it after you're back from vacation :) |
|
@AdamGleave why did you downgrade from the newest version of Atari environments? v5 has some extraordinarily large upgrades, see https://brosa.ca/blog/ale-release-v0.7 |
There is some discussion of this in #734 (comment) and araffin expressed some concerns in #572 (comment) araffin can add his own thoughts but my desiderata before upgrading would be:
I do think there are significant improvements in v5 and appreciate your & @JesseFarebro's work implementing these and simplifying Gym. But that upgrade seems better dealt with in a new PR (which you're welcome to open), whereas us lagging behind a stable release of a key dependency is more urgent to fix. |
| @@ -116,7 +116,7 @@ | |||
| # For render | |||
| "opencv-python", | |||
| # For atari games, | |||
| "atari_py==0.2.6", | |||
| "gym[atari,accept-rom-license]>=0.21", | |||
There was a problem hiding this comment.
I think this doesn't work (I remember testing it in the past), we should put autorom with accept license here
There was a problem hiding this comment.
I just tested it and it seems to work OK:
virtualenv test_venv
. ./test_venv/bin/activate
pip install -e .[extra]
then:
$ python -c 'import gym; gym.make("BreakoutNoFrameSkip-v4")'
A.L.E: Arcade Learning Environment (version 0.7.4+069f8bd)
[Powered by Stella]
IIRC it fails on some very old pip versions that don't support backtracking. I tested on pip 20.3.4 and Python 3.9, but I'm pretty sure it works on older versions (there's some discussion about this in the review).
| @@ -73,7 +73,7 @@ | |||
| packages=[package for package in find_packages() if package.startswith("stable_baselines3")], | |||
| package_data={"stable_baselines3": ["py.typed", "version.txt"]}, | |||
| install_requires=[ | |||
| "gym>=0.17,<0.20", # gym 0.20 breaks atari-py behavior | |||
| "gym>=0.21", # Remember to also update gym version in "extra" below when this changes | |||
There was a problem hiding this comment.
I would probably fixed the version until #780 is ready
Description
This is a follow-up from #705, to make it merge-able and move forward
Closes #674
Motivation and Context
Update to new gym version, some fixes were required.
Types of changes
Checklist:
make format(required)make check-codestyleandmake lint(required)make pytestandmake typeboth pass. (required)make doc(required)Note: You can run most of the checks using
make commit-checks.Note: we are using a maximum length of 127 characters per line
Changelist
setup.py(see comment)TODOs
all of which live in other repos. Are there any remaining docs that need updating in this repo?