From 104f7a107fc198380a1e0aa672e8ba90f18758cf Mon Sep 17 00:00:00 2001 From: Teddy Tennant Date: Wed, 8 Jul 2026 16:59:54 -0400 Subject: [PATCH] Reject degenerate stochastic frameskip (n, n) at construction The stochastic-frameskip validation in AtariEnv.__init__ only rejected reversed bounds (frameskip[0] > frameskip[1]), so an equal-bounds tuple such as (4, 4) passed validation. step() then calls self.np_random.integers(*self._frameskip), and numpy's integers() samples from the half-open interval [low, high); with low == high that range is empty and raises "ValueError: low >= high". The result was that construction and reset succeeded but the first step() died with a cryptic numpy error unrelated on its face to the frameskip argument. Tighten the check to frameskip[0] >= frameskip[1] so an empty range fails fast at construction, and fix the copy-paste error in the lower-bound message (it previously duplicated the bound-ordering text). --- src/ale/python/env.py | 6 +++--- tests/python/test_atari_env.py | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/ale/python/env.py b/src/ale/python/env.py index 1adaf4a00..9133f65d4 100644 --- a/src/ale/python/env.py +++ b/src/ale/python/env.py @@ -102,13 +102,13 @@ def __init__( raise error.Error( f"Invalid stochastic frameskip length of {len(frameskip)}, expected length 2." ) - elif isinstance(frameskip, tuple) and frameskip[0] > frameskip[1]: + elif isinstance(frameskip, tuple) and frameskip[0] >= frameskip[1]: raise error.Error( - "Invalid stochastic frameskip, lower bound is greater than upper bound." + "Invalid stochastic frameskip, lower bound must be strictly less than upper bound." ) elif isinstance(frameskip, tuple) and frameskip[0] <= 0: raise error.Error( - "Invalid stochastic frameskip lower bound is greater than upper bound." + "Invalid stochastic frameskip, lower bound must be positive." ) if render_mode is not None and render_mode not in {"rgb_array", "human"}: diff --git a/tests/python/test_atari_env.py b/tests/python/test_atari_env.py index cdc028508..2aa37faf2 100644 --- a/tests/python/test_atari_env.py +++ b/tests/python/test_atari_env.py @@ -239,7 +239,9 @@ def test_gym_reset_with_infos(tetris_env): assert "frame_number" in info -@pytest.mark.parametrize("frameskip", [0, -1, 4.0, (-1, 5), (0, 5), (5, 2), (1, 2, 3)]) +@pytest.mark.parametrize( + "frameskip", [0, -1, 4.0, (-1, 5), (0, 5), (5, 2), (4, 4), (1, 2, 3)] +) def test_frameskip_warnings(tetris_rom_path, frameskip): with patch("ale_py.roms.Tetris", create=True, new_callable=lambda: tetris_rom_path): with pytest.raises(gymnasium.error.Error):