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

Current repo incompatible with gym 0.24 (latest gym release) #173

Closed
jfrancis71 opened this issue Jun 3, 2022 · 6 comments
Closed

Current repo incompatible with gym 0.24 (latest gym release) #173

jfrancis71 opened this issue Jun 3, 2022 · 6 comments

Comments

@jfrancis71
Copy link
Contributor

Current repo incompatible with gym 0.24 (latest gym release):

Reproduce: (using current git repo)

import pfrl
env = pfrl.wrappers.atari_wrappers.make_atari("PongNoFrameskip-v4")
env.reset()

Error:
Traceback (most recent call last):
File "", line 1, in
File "/home/julian/pfrl/pfrl/wrappers/atari_wrappers.py", line 144, in reset
return self.env.reset(**kwargs)
File "/home/julian/pfrl/pfrl/wrappers/atari_wrappers.py", line 40, in reset
noops = self.unwrapped.np_random.randint(
AttributeError: 'numpy.random._generator.Generator' object has no attribute 'randint'

pfrl hash: d89ddf6
gym=0.24 (also same problem in 0.22,0.23)
numpy=1.22.4

Any help is welcome. Thanks,
Julian.

@jfrancis71
Copy link
Contributor Author

On further investigation:

It seems that the recent releases of gym have separated out the Atari ALE and if you install the latest gym[atari]
you current get ALE 0.7.5 and there is a change between ALE 0.7.4 and 0.7.5 which causes the above problem.

ALE 0.7.5 was released on 18 April 2022 and includes a commit which I reference here:
github link
In file: src/gym/envs/atari/environment.py
See deleted line 165: self.np_random, seed1 = seeding.np_random(seed)
replaced with line 181: self.np_random = np.random.default_rng(seed1)

My understanding is this was using a numpy RandomState object which is now deprecated (by numpy) and
the ALE maintainers have changed to using the replacement numpy.random.Generator object.

And with this new class, the member function randint has the equivalent function integers.
I have updated (in my git copy) from randint to integers and this seems to fix the problem:
File "/home/julian/pfrl/pfrl/wrappers/atari_wrappers.py", line 40, in reset
noops = self.unwrapped.np_random.randint(
AttributeError: 'numpy.random._generator.Generator' object has no attribute 'randint'

On a high level design note, our use of np_random creates a coupling to ALE; I wasn't sure from the ALE documentation if np_random is part of the ALE public API, so....it might change again?

Also please note there is a bit of a strategic aspect to this, in that if this change is made to the main PFRL repository, presumably it would mean that latest git repo would only work with the more recent versions of gym and would fail on gym=0.18.

Additionally, not directly related to this, but is related to using latest versions of gym, the rendering methods have changed, and env.render is now deprecated, and won't work in the way PFRL uses it.

I am new to Python, git and PFRL so any comments would be welcome, and I hope this is helpful,
Julian.

@muupan
Copy link
Member

muupan commented Jun 8, 2022

Thank you for reporting and investigating the issue. It is really helpful. I think it is good to support both new and old envs by using .integers only for new ones.

@jfrancis71
Copy link
Contributor Author

Hi,

I've altered the relevant lines in ./pfrl/wrappers/atari_wrappers.py to:

       if gym.__version__ >= "0.24.0":
           noops = self.unwrapped.np_random.integers(
               1, self.noop_max + 1)
       else:
           noops = self.unwrapped.np_random.randint(
               1, self.noop_max + 1)

I've tested it in both gym==0.18, gym==0.24, gym==0.24.1 .... (yes there has been a new gym released 0.24.1 only 3 days ago).
Incidentally, it was gym==0.24.0 where they bumped ale-py support from 0.7.4 to 0.7.5

I'd be happy to create a pull request for this change if that would be helpful.

jfrancis71 added a commit to jfrancis71/pfrl that referenced this issue Jun 13, 2022
…er np_random has changed from numpy RandomState to numpy Generator. See issue pfnet#173
@ummavi
Copy link
Member

ummavi commented Jun 16, 2022

Hi. Thank you so much for reporting back. A PR would be highly appreciated!

@jfrancis71
Copy link
Contributor Author

Hi,

I think I've put a pull request in. It's titled: "Fix for issue #173"

muupan added a commit that referenced this issue Sep 21, 2022
@muupan
Copy link
Member

muupan commented Sep 21, 2022

Resolved by #174

@muupan muupan closed this as completed Sep 21, 2022
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

No branches or pull requests

3 participants