Farama-Foundation / Arcade-Learning-Environment

The Arcade Learning Environment (ALE) -- a platform for AI research.
GNU General Public License v2.0
2.12k stars 420 forks source link

Atari v5 max_episode_steps does not honor frameskip setting #464

Closed hh0rva1h closed 2 years ago

hh0rva1h commented 2 years ago

https://github.com/mgbellemare/Arcade-Learning-Environment/blob/969857fc0dd2cde24b425dba950aec73dbc89109/src/python/gym.py#L184 I think instead of 108000 // 4 it should read something like 108000 // frameskip here, shouldn't it? (From the quick first look I'm not sure how you would want to access the frameskip setting though at that point.)

hh0rva1h commented 2 years ago

Problem is more difficult on second look, I guess the registration code is right the way it is, but would wish a way to correct the max_episode_steps when frameskip is set to a different value when doing gym.make().

JesseFarebro commented 2 years ago

Hi @hh0rva1h, I agree the way we currently set max_episode_steps isn't ideal as it doesn't dynamically update based on the frameskip value. The issue is that for legacy reasons we're using the default Gym wrapper to truncate the episode and it truncates based on the number of steps. Furthermore, you can only set this parameter when registering the environment so if you pass a different frameskip value when calling gym.make there's no good way to update the wrapper.

JesseFarebro commented 1 year ago

In Gym we no longer use the TimeLimit wrapper and specify the max number of frames ourselves. This solves any confusion with steps versus frames.

rfali commented 1 year ago

If anyone wants clarity on what changed in the comment by @JesseFarebro above, please see this commit. Note changes to GymFlavor v0, v4, v5 in src/python/gym.py (i.e. max_episode_steps)