Skip to content

Commit 104f7a1

Browse files
committed
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).
1 parent 4642385 commit 104f7a1

2 files changed

Lines changed: 6 additions & 4 deletions

File tree

src/ale/python/env.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,13 @@ def __init__(
102102
raise error.Error(
103103
f"Invalid stochastic frameskip length of {len(frameskip)}, expected length 2."
104104
)
105-
elif isinstance(frameskip, tuple) and frameskip[0] > frameskip[1]:
105+
elif isinstance(frameskip, tuple) and frameskip[0] >= frameskip[1]:
106106
raise error.Error(
107-
"Invalid stochastic frameskip, lower bound is greater than upper bound."
107+
"Invalid stochastic frameskip, lower bound must be strictly less than upper bound."
108108
)
109109
elif isinstance(frameskip, tuple) and frameskip[0] <= 0:
110110
raise error.Error(
111-
"Invalid stochastic frameskip lower bound is greater than upper bound."
111+
"Invalid stochastic frameskip, lower bound must be positive."
112112
)
113113

114114
if render_mode is not None and render_mode not in {"rgb_array", "human"}:

tests/python/test_atari_env.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,9 @@ def test_gym_reset_with_infos(tetris_env):
239239
assert "frame_number" in info
240240

241241

242-
@pytest.mark.parametrize("frameskip", [0, -1, 4.0, (-1, 5), (0, 5), (5, 2), (1, 2, 3)])
242+
@pytest.mark.parametrize(
243+
"frameskip", [0, -1, 4.0, (-1, 5), (0, 5), (5, 2), (4, 4), (1, 2, 3)]
244+
)
243245
def test_frameskip_warnings(tetris_rom_path, frameskip):
244246
with patch("ale_py.roms.Tetris", create=True, new_callable=lambda: tetris_rom_path):
245247
with pytest.raises(gymnasium.error.Error):

0 commit comments

Comments
 (0)