From b7aa97a81e75f79d2d405a24e1cf58058ad64b85 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Jul 2026 12:54:23 +0000 Subject: [PATCH 1/3] Make walkthrough seed opt-in and default to stochastic episodes Co-authored-by: MarcCote <660004+MarcCote@users.noreply.github.com> --- README.md | 26 +++++++++++ docs/source/tutorial_quick.rst | 6 ++- jericho/jericho.py | 80 +++++++++++++++++++++++++++++----- jericho/version.py | 2 +- tests/test_jericho.py | 4 +- tests/test_seed.py | 76 ++++++++++++++++++++++++++++++++ tools/find_walkthrough.py | 2 +- tools/test_games.py | 2 +- 8 files changed, 181 insertions(+), 17 deletions(-) create mode 100644 tests/test_seed.py diff --git a/README.md b/README.md index 6198b10a..b10b9ae4 100644 --- a/README.md +++ b/README.md @@ -43,6 +43,32 @@ python -m spacy download en_core_web_sm - [Utilities](https://jericho-py.readthedocs.io/en/latest/util.html) - [Defines](https://jericho-py.readthedocs.io/en/latest/defines.html) +## Breaking changes in Jericho 4.0 + +Prior to version 4.0, creating an environment without specifying a seed would silently +use the game's walkthrough seed (when known), making episodes deterministic. As described +in the [Jericho paper](http://arxiv.org/abs/1909.05398), a fixed random seed is a *handicap* +that should be chosen and disclosed explicitly. Starting with version 4.0: + +- `FrotzEnv(rom)` (i.e. without a seed) now uses a time-dependent seed, i.e. episodes are stochastic. +- `FrotzEnv.reset()` accepts a `use_walkthrough_seed` argument to seed the emulator with the + game's walkthrough seed, which is needed to reproduce the walkthrough. +- `FrotzEnv.walkthrough_seed` returns the game's walkthrough seed, if it is known, otherwise `None`. +- A `ImplicitRandomSeedWarning` is issued when resetting a game that has a walkthrough seed while + neither an explicit seed nor `use_walkthrough_seed` was provided. + +```python +from jericho import FrotzEnv + +env = FrotzEnv("zork1.z5") # Stochastic (time-dependent seed). +env = FrotzEnv("zork1.z5", seed=-1) # Stochastic, explicitly (no warning). +env = FrotzEnv("zork1.z5", seed=42) # Deterministic with seed 42. + +env.reset() # Uses the seed above. +env.reset(use_walkthrough_seed=True) # Deterministic, reproduces env.get_walkthrough(). +print(env.walkthrough_seed) # 12 +``` + ## Agents - [Reading Comprehension Deep Q-Network (RCDQN)](https://github.com/XiaoxiaoGuo/rcdqn) diff --git a/docs/source/tutorial_quick.rst b/docs/source/tutorial_quick.rst index cd40d222..75f69efd 100644 --- a/docs/source/tutorial_quick.rst +++ b/docs/source/tutorial_quick.rst @@ -56,6 +56,7 @@ Jericho implements a reinforcement learning interface in which the agent provide from jericho import * # Create the environment, optionally specifying a random seed + # (by default, the emulator is seeded with the current time). env = FrotzEnv("z-machine-games-master/jericho-game-suite/zork1.z5") initial_observation, info = env.reset() done = False @@ -127,12 +128,15 @@ One of the most common difficulties with parser-based text games is identifying Walkthroughs ------------ -Jericho provides walkthroughs for supported games using :meth:`jericho.FrotzEnv.get_walkthrough`. To use the walkthrough, it is necessary to reset the environment with the desired seed: +Jericho provides walkthroughs for supported games using :meth:`jericho.FrotzEnv.get_walkthrough`. To reproduce a walkthrough, it is necessary to reset the environment with the game's walkthrough seed, which is available via :attr:`jericho.FrotzEnv.walkthrough_seed`: .. code-block:: python >>> from jericho import * >>> env = FrotzEnv("z-machine-games-master/jericho-game-suite/zork1.z5") >>> walkthrough = env.get_walkthrough() + >>> env.reset(use_walkthrough_seed=True) # Equivalent to env.seed(env.walkthrough_seed); env.reset() >>> for act in walkthrough: >>> env.step(act) + +.. note:: Since Jericho 4.0, an environment created without an explicit seed is stochastic, i.e. the emulator's random number generator is seeded with the current time. Seeding the emulator (e.g. with the walkthrough seed) is a *handicap*, as defined in the `Jericho paper `_, and should be disclosed when reporting results. diff --git a/jericho/jericho.py b/jericho/jericho.py index 38e14ec3..61b45609 100644 --- a/jericho/jericho.py +++ b/jericho/jericho.py @@ -368,17 +368,26 @@ class TruncatedInputActionWarning(UserWarning): pass +class ImplicitRandomSeedWarning(UserWarning): + pass + + class FrotzEnv(): """ The Frotz Environment is a fast interface to Z-Machine games. :param story_file: Path to a Z-machine rom file (.z3/.z5/.z6/.z8). :param seed: Seed the random number generator used by the emulator. - Default: use walkthrough's seed if it exists, - otherwise use value of -1 which changes with time. + Default: -1, i.e. the emulator's random number generator is + seeded with the current time, making episodes stochastic. :type story_file: path :type seed: int + .. note:: Since Jericho 4.0, the seed needed to reproduce a game's walkthrough + is no longer used by default. To reproduce a walkthrough, either call + :meth:`jericho.FrotzEnv.reset` with `use_walkthrough_seed=True` or + provide :attr:`jericho.FrotzEnv.walkthrough_seed` as the `seed` argument. + """ def __init__(self, story_file, seed=None): self._cache = {} @@ -397,8 +406,8 @@ def load(self, story_file, seed=None): :param story_file: Path to a Z-machine rom file (.z3/.z5/.z6/.z8). :param seed: Seed the random number generator used by the emulator. - Default: use walkthrough's seed if it exists, - otherwise use value of -1 which changes with time. + Default: -1, i.e. the emulator's random number generator is + seeded with the current time, making episodes stochastic. :type story_file: path :type seed: int ''' @@ -434,30 +443,79 @@ def seed(self, seed=None): Changes seed used for the emulator's random number generator. :param seed: Seed the random number generator used by the emulator. - Default: use walkthrough's seed if it exists, - otherwise use value of -1 which changes with time. + Default: -1, i.e. the emulator's random number generator is + seeded with the current time, making episodes stochastic. :returns: The value of the seed. .. note:: :meth:`jericho.FrotzEnv.reset()` must be called before the seed takes effect. + .. note:: Since Jericho 4.0, calling this method without a seed no longer + silently uses the game's walkthrough seed. Use + :attr:`jericho.FrotzEnv.walkthrough_seed` or + :meth:`jericho.FrotzEnv.reset` with `use_walkthrough_seed=True` + to reproduce a walkthrough. + + ''' + self._seed_is_explicit = seed is not None + self._seed = seed if seed is not None else -1 + return self._seed + + @property + def walkthrough_seed(self): + ''' + Seed needed to reproduce this game's walkthrough, if it is known. + + :returns: The walkthrough's seed, or `None` if the game has no known walkthrough seed. + + :Example: + + >>> import jericho + >>> env = jericho.FrotzEnv('zork1.z5') + >>> env.walkthrough_seed + 12 + >>> env.reset(use_walkthrough_seed=True) # Same as env.seed(env.walkthrough_seed); env.reset() + ''' - seed = seed or self.bindings.get('seed', -1) - self._seed = seed - return seed + return self.bindings.get('seed') - def reset(self): + def reset(self, use_walkthrough_seed=False): ''' Resets the game. :param use_walkthrough_seed: Seed the emulator to reproduce the walkthrough. + Default: `False`, i.e. use the seed set with + :meth:`jericho.FrotzEnv.seed` (a time-dependent + seed, unless one was explicitly provided). + :type use_walkthrough_seed: bool :returns: A tuple containing the initial observation,\ and a dictionary of info. :rtype: string, dictionary + .. note:: Using `use_walkthrough_seed=True` makes the game deterministic. + As described in the Jericho paper, this is a *handicap* that + should be disclosed when reporting results. + ''' + seed = self._seed + if use_walkthrough_seed: + if self.walkthrough_seed is None: + msg = ("No walkthrough seed is known for game '{}'," + " using a time-dependent seed instead.").format(self.story_file.decode()) + warnings.warn(msg, UnsupportedGameWarning) + else: + seed = self.walkthrough_seed + + elif not self._seed_is_explicit and self.walkthrough_seed is not None: + msg = ("Since Jericho 4.0, the walkthrough seed ({}) of game '{}' is no longer used" + " by default, i.e. this episode is stochastic (time-dependent seed)." + " Call reset(use_walkthrough_seed=True) to reproduce the walkthrough," + " or provide an explicit seed (e.g. FrotzEnv(rom, seed=-1)) to silence" + " this warning.").format(self.walkthrough_seed, self.story_file.decode()) + warnings.warn(msg, ImplicitRandomSeedWarning) + self.close() rom, _, _ = self._cache[self.story_file.decode()] - obs_ini = self.frotz_lib.setup(self.story_file, self._seed, rom, len(rom)).decode('cp1252') + obs_ini = self.frotz_lib.setup(self.story_file, seed, rom, len(rom)).decode('cp1252') score = self.frotz_lib.get_score() return obs_ini, {'moves':self.get_moves(), 'score':score} diff --git a/jericho/version.py b/jericho/version.py index 310a75df..d6497a81 100644 --- a/jericho/version.py +++ b/jericho/version.py @@ -1 +1 @@ -__version__ = '3.3.1' +__version__ = '4.0.0' diff --git a/tests/test_jericho.py b/tests/test_jericho.py index 12f8b22d..1e3a0ef2 100644 --- a/tests/test_jericho.py +++ b/tests/test_jericho.py @@ -80,12 +80,12 @@ def _get_mem(): def test_copy(): rom = pjoin(DATA_PATH, "905.z5") env = jericho.FrotzEnv(rom) - env.reset() + env.reset(use_walkthrough_seed=True) walkthrough = env.get_walkthrough() expected = [env.step(act) for act in walkthrough] - env.reset() + env.reset(use_walkthrough_seed=True) for i, act in enumerate(walkthrough): obs, rew, done, info = env.step(act) diff --git a/tests/test_seed.py b/tests/test_seed.py new file mode 100644 index 00000000..a1c4f1f4 --- /dev/null +++ b/tests/test_seed.py @@ -0,0 +1,76 @@ +import os +import warnings +from os.path import join as pjoin + +import pytest + +import jericho + + +DATA_PATH = os.path.abspath(pjoin(__file__, '..', "data")) + + +def test_default_seed_is_time_dependent(): + # By default, the walkthrough seed should *not* be used silently. + rom = pjoin(DATA_PATH, "905.z5") + env = jericho.FrotzEnv(rom) + assert env._seed == -1 + assert env.seed() == -1 + + +def test_explicit_seed(): + rom = pjoin(DATA_PATH, "905.z5") + env = jericho.FrotzEnv(rom, seed=42) + assert env._seed == 42 + + # Zero is a valid seed. + assert env.seed(0) == 0 + assert env._seed == 0 + + +def test_walkthrough_seed_property(): + env = jericho.FrotzEnv(pjoin(DATA_PATH, "905.z5")) + assert env.walkthrough_seed == env.bindings['seed'] + + # Games without bindings have no walkthrough seed. + env = jericho.FrotzEnv(pjoin(DATA_PATH, "tw-game.z8")) + assert env.walkthrough_seed is None + + +def test_warning_when_using_implicit_random_seed(): + rom = pjoin(DATA_PATH, "905.z5") + env = jericho.FrotzEnv(rom) + + with pytest.warns(jericho.ImplicitRandomSeedWarning): + env.reset() + + # No warning when the choice is explicit. + with warnings.catch_warnings(): + warnings.simplefilter("error") + env.reset(use_walkthrough_seed=True) + jericho.FrotzEnv(rom, seed=-1).reset() + jericho.FrotzEnv(rom, seed=env.walkthrough_seed).reset() + + # No warning for games without a walkthrough seed. + with warnings.catch_warnings(): + warnings.simplefilter("error") + jericho.FrotzEnv(pjoin(DATA_PATH, "tw-game.z8")).reset() + + +def test_reset_with_walkthrough_seed_but_no_bindings(): + env = jericho.FrotzEnv(pjoin(DATA_PATH, "tw-game.z8")) + with pytest.warns(jericho.UnsupportedGameWarning): + env.reset(use_walkthrough_seed=True) + + +def test_walkthrough_is_reproducible_with_walkthrough_seed(): + rom = pjoin(DATA_PATH, "905.z5") + env = jericho.FrotzEnv(rom) + walkthrough = env.get_walkthrough() + + env.reset(use_walkthrough_seed=True) + for act in walkthrough: + obs, rew, done, info = env.step(act) + + assert done + assert info["score"] == env.get_max_score() diff --git a/tools/find_walkthrough.py b/tools/find_walkthrough.py index 7def8f20..23a27254 100644 --- a/tools/find_walkthrough.py +++ b/tools/find_walkthrough.py @@ -23,7 +23,7 @@ def parse_args(): history = [] env = jericho.FrotzEnv(args.filename) -obs, info = env.reset() +obs, info = env.reset(use_walkthrough_seed=True) history.append(env.get_state()) diff --git a/tools/test_games.py b/tools/test_games.py index 1d38b18c..706379cf 100644 --- a/tools/test_games.py +++ b/tools/test_games.py @@ -32,7 +32,7 @@ def parse_args(): print(colored("SKIP\tMissing walkthrough", 'yellow')) continue - env.reset() + env.reset(use_walkthrough_seed=True) #walkthrough = bindings['walkthrough'].split('/') for cmd in env.get_walkthrough(): From 5a02ce63646713d980a6bcac3c1e00bc2640a1d8 Mon Sep 17 00:00:00 2001 From: Alessandro Bahgat Date: Wed, 29 Jul 2026 22:03:03 +0000 Subject: [PATCH 2/3] Warn on first implicit episode, validate seeds, strengthen seed tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the opt-in walkthrough-seed change: - Emit ImplicitRandomSeedWarning once per env, at the start of the first episode played without an explicit seeding choice (reset() or, since the env is playable right after construction, a bare step()). Warning only in reset() missed step-without-reset callers, while warning at load time would flag correct usage such as FrotzEnv(rom) followed by reset(use_walkthrough_seed=True) — this covers both. The warned flag is set before warning so users running with -W error get a single exception on a fully constructed env rather than one per reset forever, and stacklevel points the warning at the caller's code. - Treat any direct call to seed() as an explicit seeding choice, including seed() without arguments (a deliberate request for a time-dependent seed); it suppresses the warning for later episodes. - Validate seeds fit in a signed 32-bit int. The emulator receives the seed as a C int, so e.g. seed=2**32-1 (a common shape, like np.random.randint(2**32)) silently wrapped to the -1 time-dependent sentinel, making an explicitly seeded env stochastic. - reset(use_walkthrough_seed=True) on a game with no walkthrough seed warns (UnsupportedGameWarning) and proceeds with the environment's configured seed. The warning previously claimed a time-dependent seed was used, which was wrong whenever an explicit seed was set; it now reports the actual behavior, and a test pins it. - Preserve seed bookkeeping in copy(): a fork no longer silently counts as explicitly seeded, and its docstring clarifies that reset() on a fork does not reproduce a use_walkthrough_seed=True episode. - Strengthen tests. The prior reproducibility check could not catch a broken use_walkthrough_seed flag: 905's walkthrough is RNG-independent (any seed reaches max score) and time seeds have one-second resolution, so back-to-back seeded resets matched by accident. The walkthrough-seeded episode is now cross-validated against an env explicitly constructed with walkthrough_seed. Also new: constructor- episode determinism/stochasticity (load-time seeding was untested), step-without-reset warning, once-per-env warning, seed range/type validation, fallback-seed behavior, and set_state() restoring RNG across envs (the invariant keeping parallel get_valid_actions workers sound now that they no longer share a walkthrough seed). - Docs: correct the get_state/set_state state-tuple docstrings (9 elements, not 7), drop the inaccurate 'equivalent to seed(); reset()' claim (the flag is episode-only), document that reset(True) does not modify the stored seed, and add a README note that time-dependent seeds have one-second resolution (parallel envs created in the same second play identical episodes — pass distinct explicit seeds). - Make the repo's own tests seed explicitly where seeding is not what they test. --- README.md | 14 ++- docs/source/tutorial_quick.rst | 2 +- jericho/jericho.py | 100 ++++++++++++++++--- tests/test_jericho.py | 12 +-- tests/test_seed.py | 172 ++++++++++++++++++++++++++++++--- 5 files changed, 259 insertions(+), 41 deletions(-) diff --git a/README.md b/README.md index b10b9ae4..9a1adee9 100644 --- a/README.md +++ b/README.md @@ -54,8 +54,18 @@ that should be chosen and disclosed explicitly. Starting with version 4.0: - `FrotzEnv.reset()` accepts a `use_walkthrough_seed` argument to seed the emulator with the game's walkthrough seed, which is needed to reproduce the walkthrough. - `FrotzEnv.walkthrough_seed` returns the game's walkthrough seed, if it is known, otherwise `None`. -- A `ImplicitRandomSeedWarning` is issued when resetting a game that has a walkthrough seed while - neither an explicit seed nor `use_walkthrough_seed` was provided. +- An `ImplicitRandomSeedWarning` is issued (once per environment) when the first episode of a + game that has a walkthrough seed begins — via `reset()` or a direct `step()` — without an + explicit seeding choice. Providing any seed (e.g. `seed=-1` for time-dependent randomness), + calling `env.seed()`, or resetting with `use_walkthrough_seed=True` silences it. + +To keep the old behavior (e.g. to reproduce results published with Jericho ≤ 3.x), either pin +`pip install 'jericho<4'` or seed explicitly: `env.seed(env.walkthrough_seed)` before `env.reset()`. + +> [!NOTE] +> The time-dependent seed has one-second resolution, so unseeded environments created within +> the same second play identical episodes. For parallel or vectorized runs, pass a distinct +> explicit seed to each environment. ```python from jericho import FrotzEnv diff --git a/docs/source/tutorial_quick.rst b/docs/source/tutorial_quick.rst index 75f69efd..dd6d4730 100644 --- a/docs/source/tutorial_quick.rst +++ b/docs/source/tutorial_quick.rst @@ -135,7 +135,7 @@ Jericho provides walkthroughs for supported games using :meth:`jericho.FrotzEnv. >>> from jericho import * >>> env = FrotzEnv("z-machine-games-master/jericho-game-suite/zork1.z5") >>> walkthrough = env.get_walkthrough() - >>> env.reset(use_walkthrough_seed=True) # Equivalent to env.seed(env.walkthrough_seed); env.reset() + >>> env.reset(use_walkthrough_seed=True) # Applies the walkthrough seed to this episode only. >>> for act in walkthrough: >>> env.step(act) diff --git a/jericho/jericho.py b/jericho/jericho.py index 61b45609..fc8cae26 100644 --- a/jericho/jericho.py +++ b/jericho/jericho.py @@ -17,6 +17,7 @@ import os import shutil import tempfile +import operator import warnings import hashlib @@ -372,6 +373,22 @@ class ImplicitRandomSeedWarning(UserWarning): pass +def _resolve_seed(seed): + ''' + Resolves a user-provided seed to the int handed to the emulator. + + The emulator receives the seed as a C int; without a range check, a value + like 2**32-1 (e.g. from np.random.randint(2**32)) would silently wrap to + the -1 "time-dependent" sentinel, making an explicitly seeded env stochastic. + ''' + if seed is None: + return -1 + seed = operator.index(seed) # Accepts any integer type; rejects e.g. floats. + if not -2**31 <= seed < 2**31: + raise ValueError("seed must fit in a signed 32-bit integer, got {}.".format(seed)) + return seed + + class FrotzEnv(): """ The Frotz Environment is a fast interface to Z-Machine games. @@ -434,10 +451,40 @@ def load(self, story_file, seed=None): rom, self._bindings, self.act_gen = self._cache[story_file] - self.seed(seed) + # Track seed explicitness here rather than via seed(): a direct call + # to seed() is always an explicit choice, but the constructor default + # (seed=None) is not. + self._seed_is_explicit = seed is not None + self._seed = _resolve_seed(seed) + self._warned_implicit_seed = False + self._episode_seed_implicit = not self._seed_is_explicit self.frotz_lib.setup(self.story_file, self._seed, rom, len(rom)) self.player_obj_num = self.frotz_lib.get_self_object_num() + def _maybe_warn_implicit_seed(self, stacklevel): + ''' + Warns (at most once per loaded game) when an episode is played without + an explicit seeding choice for a game whose walkthrough seed would + have been silently applied prior to Jericho 4.0. Called at the start + of the first episode interaction — reset() or, since stepping is + possible without calling reset(), the first step() — so that correct + usage such as `FrotzEnv(rom)` followed by + `reset(use_walkthrough_seed=True)` is never flagged. + ''' + if not self._episode_seed_implicit or self._warned_implicit_seed: + return + if self.walkthrough_seed is None: + return + # Mark as warned *before* warning: under warnings.simplefilter("error") + # the user gets a single exception, not one per reset()/step() forever. + self._warned_implicit_seed = True + msg = ("Since Jericho 4.0, the walkthrough seed ({}) of game '{}' is no longer used" + " by default, i.e. this episode is stochastic (time-dependent seed)." + " Call reset(use_walkthrough_seed=True) to reproduce the walkthrough," + " or make stochasticity explicit (e.g. FrotzEnv(rom, seed=-1) or env.seed(-1))" + " to silence this warning.").format(self.walkthrough_seed, self.story_file.decode()) + warnings.warn(msg, ImplicitRandomSeedWarning, stacklevel=stacklevel) + def seed(self, seed=None): ''' Changes seed used for the emulator's random number generator. @@ -455,9 +502,14 @@ def seed(self, seed=None): :meth:`jericho.FrotzEnv.reset` with `use_walkthrough_seed=True` to reproduce a walkthrough. + .. note:: Calling this method counts as an explicit seeding choice, even + without an argument (i.e. deliberately requesting a + time-dependent seed), so subsequent episodes do not raise + :class:`jericho.ImplicitRandomSeedWarning`. + ''' - self._seed_is_explicit = seed is not None - self._seed = seed if seed is not None else -1 + self._seed_is_explicit = True + self._seed = _resolve_seed(seed) return self._seed @property @@ -473,7 +525,7 @@ def walkthrough_seed(self): >>> env = jericho.FrotzEnv('zork1.z5') >>> env.walkthrough_seed 12 - >>> env.reset(use_walkthrough_seed=True) # Same as env.seed(env.walkthrough_seed); env.reset() + >>> env.reset(use_walkthrough_seed=True) # Applies the walkthrough seed to this episode only. ''' return self.bindings.get('seed') @@ -495,23 +547,24 @@ def reset(self, use_walkthrough_seed=False): As described in the Jericho paper, this is a *handicap* that should be disclosed when reporting results. + .. note:: `use_walkthrough_seed=True` applies to this episode only: it does + not modify the seed set with :meth:`jericho.FrotzEnv.seed`, so a + subsequent plain `reset()` reverts to that seed. To make the + walkthrough seed persistent, use `env.seed(env.walkthrough_seed)`. + ''' seed = self._seed if use_walkthrough_seed: if self.walkthrough_seed is None: msg = ("No walkthrough seed is known for game '{}'," - " using a time-dependent seed instead.").format(self.story_file.decode()) - warnings.warn(msg, UnsupportedGameWarning) + " using the environment's seed instead.").format(self.story_file.decode()) + warnings.warn(msg, UnsupportedGameWarning, stacklevel=2) else: seed = self.walkthrough_seed - elif not self._seed_is_explicit and self.walkthrough_seed is not None: - msg = ("Since Jericho 4.0, the walkthrough seed ({}) of game '{}' is no longer used" - " by default, i.e. this episode is stochastic (time-dependent seed)." - " Call reset(use_walkthrough_seed=True) to reproduce the walkthrough," - " or provide an explicit seed (e.g. FrotzEnv(rom, seed=-1)) to silence" - " this warning.").format(self.walkthrough_seed, self.story_file.decode()) - warnings.warn(msg, ImplicitRandomSeedWarning) + episode_explicit = self._seed_is_explicit or (use_walkthrough_seed and self.walkthrough_seed is not None) + self._episode_seed_implicit = not episode_explicit + self._maybe_warn_implicit_seed(stacklevel=3) self.close() rom, _, _ = self._cache[self.story_file.decode()] @@ -534,6 +587,9 @@ def step(self, action): Note: - The action is converted to bytes and truncated to 198 characters. ''' + # The env is playable without calling reset() first, so the implicit-seed + # warning must also cover episodes that begin with a step(). + self._maybe_warn_implicit_seed(stacklevel=3) action_bytes = action.encode('utf-8') if len(action_bytes) > INPUT_BUFFER_SIZE: action_bytes = action_bytes[:INPUT_BUFFER_SIZE] @@ -637,7 +693,7 @@ def set_state(self, state): ''' Sets the game's internal state. - :param state: Tuple of (ram, stack, pc, sp, fp, frame_count, rng) as\ + :param state: Tuple of (ram, stack, pc, sp, fp, frame_count, opcode, rng, narrative) as\ obtained by :meth:`jericho.FrotzEnv.get_state`. :type state: tuple @@ -665,7 +721,7 @@ def get_state(self): Returns the internal game state. This state can be subsequently restored using :meth:`jericho.FrotzEnv.set_state`. - :returns: Tuple of (ram, stack, pc, sp, fp, frame_count, rng). + :returns: Tuple of (ram, stack, pc, sp, fp, frame_count, opcode, rng, narrative). >>> from jericho import * >>> env = FrotzEnv(rom_path) @@ -694,9 +750,21 @@ def get_max_score(self): return self.frotz_lib.get_max_score() def copy(self): - ''' Forks this FrotzEnv instance. ''' + ''' Forks this FrotzEnv instance. + + The copy replays the current game faithfully (the emulator's RNG + registers are part of the copied state), but like the original, a + subsequent :meth:`jericho.FrotzEnv.reset` uses the seed set with + :meth:`jericho.FrotzEnv.seed` — not the seed of the episode being + copied, if that episode was started with `reset(use_walkthrough_seed=True)`. + ''' state = self.get_state() env = FrotzEnv(self.story_file.decode(), seed=self._seed) + # Passing seed= above would make the copy count as explicitly seeded; + # carry over the original's bookkeeping instead. + env._seed_is_explicit = self._seed_is_explicit + env._warned_implicit_seed = self._warned_implicit_seed + env._episode_seed_implicit = self._episode_seed_implicit env.set_state(state) return env diff --git a/tests/test_jericho.py b/tests/test_jericho.py index 1e3a0ef2..c8d2ac8b 100644 --- a/tests/test_jericho.py +++ b/tests/test_jericho.py @@ -14,7 +14,7 @@ def test_multiple_instances(): gamefile2 = pjoin(DATA_PATH, "tw-game.z8") # Make sure both frotz_lib have different handles. - env1 = jericho.FrotzEnv(gamefile1) + env1 = jericho.FrotzEnv(gamefile1, seed=-1) env2 = jericho.FrotzEnv(gamefile2) assert env1.frotz_lib._handle != env2.frotz_lib._handle @@ -47,7 +47,7 @@ def _get_mem(): unit = 1024 * 1024 gamefile1 = pjoin(DATA_PATH, "905.z5") - env1 = jericho.FrotzEnv(gamefile1) + env1 = jericho.FrotzEnv(gamefile1, seed=-1) env1.reset() del env1 @@ -55,7 +55,7 @@ def _get_mem(): print('Memory usage: {:.1f}MB'.format(mem_start / unit)) for _ in range(1000): # Make sure we don't have memory leak. - env1 = jericho.FrotzEnv(gamefile1) + env1 = jericho.FrotzEnv(gamefile1, seed=-1) env1.reset() del env1 @@ -64,7 +64,7 @@ def _get_mem(): mem_mid / unit, (mem_mid-mem_start) / unit )) for _ in range(1000): - env1 = jericho.FrotzEnv(gamefile1) + env1 = jericho.FrotzEnv(gamefile1, seed=-1) env1.reset() del env1 @@ -113,7 +113,7 @@ def test_saving_opcode_in_state(): ] rom = pjoin(DATA_PATH, "roms", "yomomma.z8") - env = jericho.FrotzEnv(rom) + env = jericho.FrotzEnv(rom, seed=-1) env.reset() state = None @@ -129,7 +129,7 @@ def test_saving_opcode_in_state(): def test_very_long_action(): rom = pjoin(DATA_PATH, "905.z5") - env = jericho.FrotzEnv(rom) + env = jericho.FrotzEnv(rom, seed=-1) env.reset() long_command = "It's a " + "very " * 36 + "long action!" diff --git a/tests/test_seed.py b/tests/test_seed.py index a1c4f1f4..f29f90c6 100644 --- a/tests/test_seed.py +++ b/tests/test_seed.py @@ -1,4 +1,5 @@ import os +import time import warnings from os.path import join as pjoin @@ -8,19 +9,32 @@ DATA_PATH = os.path.abspath(pjoin(__file__, '..', "data")) +ROM = pjoin(DATA_PATH, "905.z5") +ROM_NO_BINDINGS = pjoin(DATA_PATH, "tw-game.z8") + + +def _rng_state(env): + """ The emulator's RNG registers (the same ones get_state() captures). """ + lib = env.frotz_lib + return (lib.getRngA(), lib.getRngInterval(), lib.getRngCounter()) + + +def _quiet_env(*args, **kwargs): + """ Builds an env, ignoring the transitional implicit-seed warning. """ + with warnings.catch_warnings(): + warnings.simplefilter("ignore", jericho.ImplicitRandomSeedWarning) + return jericho.FrotzEnv(*args, **kwargs) def test_default_seed_is_time_dependent(): # By default, the walkthrough seed should *not* be used silently. - rom = pjoin(DATA_PATH, "905.z5") - env = jericho.FrotzEnv(rom) + env = _quiet_env(ROM) assert env._seed == -1 assert env.seed() == -1 def test_explicit_seed(): - rom = pjoin(DATA_PATH, "905.z5") - env = jericho.FrotzEnv(rom, seed=42) + env = jericho.FrotzEnv(ROM, seed=42) assert env._seed == 42 # Zero is a valid seed. @@ -28,44 +42,153 @@ def test_explicit_seed(): assert env._seed == 0 +def test_seed_validation(): + env = jericho.FrotzEnv(ROM, seed=42) + + # The emulator takes a C int; values that don't fit must not silently wrap. + # E.g. 2**32-1 would wrap to the -1 "time-dependent" sentinel, silently + # making an explicitly seeded env stochastic. + for bad in (2**32 - 1, 2**31, -2**31 - 1): + with pytest.raises(ValueError): + env.seed(bad) + with pytest.raises(ValueError): + jericho.FrotzEnv(ROM, seed=bad) + + with pytest.raises(TypeError): + env.seed(1.5) + + assert env.seed(2**31 - 1) == 2**31 - 1 + assert env.seed(-2**31) == -2**31 + + def test_walkthrough_seed_property(): - env = jericho.FrotzEnv(pjoin(DATA_PATH, "905.z5")) + env = _quiet_env(ROM) assert env.walkthrough_seed == env.bindings['seed'] # Games without bindings have no walkthrough seed. - env = jericho.FrotzEnv(pjoin(DATA_PATH, "tw-game.z8")) + env = jericho.FrotzEnv(ROM_NO_BINDINGS) assert env.walkthrough_seed is None -def test_warning_when_using_implicit_random_seed(): - rom = pjoin(DATA_PATH, "905.z5") - env = jericho.FrotzEnv(rom) +def test_constructor_episode_uses_explicit_seed(): + # The episode set up at load time (playable without reset()) must honor + # the constructor seed. + rng1 = _rng_state(jericho.FrotzEnv(ROM, seed=1234)) + rng2 = _rng_state(jericho.FrotzEnv(ROM, seed=1234)) + rng3 = _rng_state(jericho.FrotzEnv(ROM, seed=4321)) + assert rng1 == rng2 + assert rng1 != rng3 + +def test_constructor_episode_is_stochastic(): + # ...and without a seed it must not fall back to the walkthrough seed. + env1 = _quiet_env(ROM) + time.sleep(1.1) # The time-dependent seed has one-second resolution. + env2 = _quiet_env(ROM) + assert _rng_state(env1) != _rng_state(env2) + + +def test_unseeded_resets_are_stochastic(): + env = _quiet_env(ROM) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", jericho.ImplicitRandomSeedWarning) + env.reset() + rng1 = _rng_state(env) + time.sleep(1.1) + env.reset() + assert _rng_state(env) != rng1 + + +def test_warning_when_using_implicit_random_seed(): + # Constructing is silent; the warning fires when the first episode is + # actually played without an explicit seeding choice... + env = jericho.FrotzEnv(ROM) with pytest.warns(jericho.ImplicitRandomSeedWarning): env.reset() + # ...and only once per environment. + with warnings.catch_warnings(): + warnings.simplefilter("error") + env.reset() + + # The env is playable without reset(), so a bare step() must warn too. + env = jericho.FrotzEnv(ROM) + with pytest.warns(jericho.ImplicitRandomSeedWarning): + env.step('look') + # No warning when the choice is explicit. with warnings.catch_warnings(): warnings.simplefilter("error") - env.reset(use_walkthrough_seed=True) - jericho.FrotzEnv(rom, seed=-1).reset() - jericho.FrotzEnv(rom, seed=env.walkthrough_seed).reset() + jericho.FrotzEnv(ROM, seed=-1).reset() + jericho.FrotzEnv(ROM, seed=0).reset() + jericho.FrotzEnv(ROM).reset(use_walkthrough_seed=True) + + env = jericho.FrotzEnv(ROM) + env.seed() # Deliberate request for time-dependent randomness. + env.reset() # No warning for games without a walkthrough seed. with warnings.catch_warnings(): warnings.simplefilter("error") - jericho.FrotzEnv(pjoin(DATA_PATH, "tw-game.z8")).reset() + jericho.FrotzEnv(ROM_NO_BINDINGS).reset() + + +def test_copy_preserves_seed_bookkeeping(): + env = _quiet_env(ROM) + fork = env.copy() + + # The fork replays the parent's episode faithfully... + assert _rng_state(fork) == _rng_state(env) + # ...and keeps the parent's seed bookkeeping instead of silently becoming + # "explicitly seeded" via the seed= constructor argument copy() uses. + assert fork._seed_is_explicit == env._seed_is_explicit + assert fork._warned_implicit_seed == env._warned_implicit_seed + assert fork._episode_seed_implicit == env._episode_seed_implicit + + # A fork of an implicitly seeded env warns on its first episode, like the parent. + with pytest.warns(jericho.ImplicitRandomSeedWarning): + fork.step('look') + + fork = jericho.FrotzEnv(ROM, seed=42).copy() + assert fork._seed_is_explicit def test_reset_with_walkthrough_seed_but_no_bindings(): - env = jericho.FrotzEnv(pjoin(DATA_PATH, "tw-game.z8")) + env = jericho.FrotzEnv(ROM_NO_BINDINGS, seed=42) + with pytest.warns(jericho.UnsupportedGameWarning, match="environment's seed"): + env.reset(use_walkthrough_seed=True) + # It falls back to the env's seed (not a time-dependent one). + rng1 = _rng_state(env) + time.sleep(1.1) with pytest.warns(jericho.UnsupportedGameWarning): env.reset(use_walkthrough_seed=True) + assert _rng_state(env) == rng1 + + +def test_reset_with_walkthrough_seed_applies_the_walkthrough_seed(): + # Cross-validate the flag against an env explicitly seeded with the + # walkthrough seed. Score-based checks or back-to-back seeded resets can't + # catch a broken flag: 905's walkthrough is RNG-independent, and two + # time-based seeds drawn within the same second are identical anyway. + env = jericho.FrotzEnv(ROM) + walkthrough = env.get_walkthrough() + prefix = walkthrough[:5] + + env.reset(use_walkthrough_seed=True) + for act in prefix: + env.step(act) + + ref = _quiet_env(ROM, seed=env.walkthrough_seed) + ref.reset() + for act in prefix: + ref.step(act) + + assert _rng_state(env) == _rng_state(ref) + assert env.get_world_state_hash() == ref.get_world_state_hash() def test_walkthrough_is_reproducible_with_walkthrough_seed(): - rom = pjoin(DATA_PATH, "905.z5") - env = jericho.FrotzEnv(rom) + env = jericho.FrotzEnv(ROM) walkthrough = env.get_walkthrough() env.reset(use_walkthrough_seed=True) @@ -74,3 +197,20 @@ def test_walkthrough_is_reproducible_with_walkthrough_seed(): assert done assert info["score"] == env.get_max_score() + + +def test_set_state_restores_rng_across_envs(): + # get_valid_actions(use_parallel=True) forks worker envs which sync via + # set_state(); this is only sound if set_state restores the RNG registers, + # since the workers no longer share the parent's (walkthrough) seed. + env = _quiet_env(ROM) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", jericho.ImplicitRandomSeedWarning) + env.reset() + for act in env.get_walkthrough()[:3]: + env.step(act) + state = env.get_state() + + worker = jericho.FrotzEnv(ROM, seed=-1) # Different time-dependent seed. + worker.set_state(state) + assert _rng_state(worker) == _rng_state(env) From e861c9bf477e276f5f5ce4b36379b274c80500d3 Mon Sep 17 00:00:00 2001 From: Alessandro Bahgat Date: Wed, 5 Aug 2026 20:42:11 +0000 Subject: [PATCH 3/3] Draw per-episode seeds in Python and raise when the walkthrough seed is unknown Addresses code review threads on #88: - Resolve the stochastic sentinel (-1) into a concrete 31-bit seed drawn from OS entropy instead of letting the emulator seed itself from time(0), whose one-second resolution made unseeded envs created in the same second (e.g. parallel workers) play identical episodes. The seed used is reported via the new FrotzEnv.episode_seed property, in reset()'s info dict, and in the implicit-seed warning, so any episode can be reproduced after the fact. - reset(use_walkthrough_seed=True) now raises ValueError when the game has no known walkthrough seed, instead of warning and silently substituting the environment's seed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019LPt5yAyGWYcbFLe9r1H5M --- README.md | 20 +++---- docs/source/tutorial_quick.rst | 4 +- jericho/jericho.py | 101 +++++++++++++++++++++++++-------- tests/test_seed.py | 55 +++++++++++++----- 4 files changed, 130 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 9a1adee9..ca684c12 100644 --- a/README.md +++ b/README.md @@ -50,31 +50,31 @@ use the game's walkthrough seed (when known), making episodes deterministic. As in the [Jericho paper](http://arxiv.org/abs/1909.05398), a fixed random seed is a *handicap* that should be chosen and disclosed explicitly. Starting with version 4.0: -- `FrotzEnv(rom)` (i.e. without a seed) now uses a time-dependent seed, i.e. episodes are stochastic. +- `FrotzEnv(rom)` (i.e. without a seed) is now stochastic: a fresh random seed is drawn for + each episode. The seed actually used is reported in `reset()`'s info dict and as + `FrotzEnv.episode_seed`, so any episode can be reproduced after the fact. - `FrotzEnv.reset()` accepts a `use_walkthrough_seed` argument to seed the emulator with the - game's walkthrough seed, which is needed to reproduce the walkthrough. + game's walkthrough seed, which is needed to reproduce the walkthrough. It raises `ValueError` + if the game has no known walkthrough seed (check `env.walkthrough_seed is None`). - `FrotzEnv.walkthrough_seed` returns the game's walkthrough seed, if it is known, otherwise `None`. - An `ImplicitRandomSeedWarning` is issued (once per environment) when the first episode of a game that has a walkthrough seed begins — via `reset()` or a direct `step()` — without an - explicit seeding choice. Providing any seed (e.g. `seed=-1` for time-dependent randomness), + explicit seeding choice. Providing any seed (e.g. `seed=-1` to explicitly request random episodes), calling `env.seed()`, or resetting with `use_walkthrough_seed=True` silences it. To keep the old behavior (e.g. to reproduce results published with Jericho ≤ 3.x), either pin `pip install 'jericho<4'` or seed explicitly: `env.seed(env.walkthrough_seed)` before `env.reset()`. -> [!NOTE] -> The time-dependent seed has one-second resolution, so unseeded environments created within -> the same second play identical episodes. For parallel or vectorized runs, pass a distinct -> explicit seed to each environment. - ```python from jericho import FrotzEnv -env = FrotzEnv("zork1.z5") # Stochastic (time-dependent seed). +env = FrotzEnv("zork1.z5") # Stochastic (a random seed is drawn per episode). +obs, info = env.reset() # info['seed'] (also env.episode_seed) is the drawn seed. +replay = FrotzEnv("zork1.z5", seed=info['seed']) # Reproduces the episode above. + env = FrotzEnv("zork1.z5", seed=-1) # Stochastic, explicitly (no warning). env = FrotzEnv("zork1.z5", seed=42) # Deterministic with seed 42. -env.reset() # Uses the seed above. env.reset(use_walkthrough_seed=True) # Deterministic, reproduces env.get_walkthrough(). print(env.walkthrough_seed) # 12 ``` diff --git a/docs/source/tutorial_quick.rst b/docs/source/tutorial_quick.rst index dd6d4730..289e157c 100644 --- a/docs/source/tutorial_quick.rst +++ b/docs/source/tutorial_quick.rst @@ -56,7 +56,7 @@ Jericho implements a reinforcement learning interface in which the agent provide from jericho import * # Create the environment, optionally specifying a random seed - # (by default, the emulator is seeded with the current time). + # (by default, a fresh random seed is drawn for each episode). env = FrotzEnv("z-machine-games-master/jericho-game-suite/zork1.z5") initial_observation, info = env.reset() done = False @@ -139,4 +139,4 @@ Jericho provides walkthroughs for supported games using :meth:`jericho.FrotzEnv. >>> for act in walkthrough: >>> env.step(act) -.. note:: Since Jericho 4.0, an environment created without an explicit seed is stochastic, i.e. the emulator's random number generator is seeded with the current time. Seeding the emulator (e.g. with the walkthrough seed) is a *handicap*, as defined in the `Jericho paper `_, and should be disclosed when reporting results. +.. note:: Since Jericho 4.0, an environment created without an explicit seed is stochastic: a fresh random seed is drawn for each episode. The seed actually used is reported in the info dict returned by :meth:`jericho.FrotzEnv.reset` and via :attr:`jericho.FrotzEnv.episode_seed`, so any episode can be reproduced after the fact. Seeding the emulator (e.g. with the walkthrough seed) is a *handicap*, as defined in the `Jericho paper `_, and should be disclosed when reporting results. diff --git a/jericho/jericho.py b/jericho/jericho.py index fc8cae26..caab4a12 100644 --- a/jericho/jericho.py +++ b/jericho/jericho.py @@ -15,6 +15,7 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. import os +import random import shutil import tempfile import operator @@ -375,11 +376,11 @@ class ImplicitRandomSeedWarning(UserWarning): def _resolve_seed(seed): ''' - Resolves a user-provided seed to the int handed to the emulator. + Resolves a user-provided seed to the int stored on the environment. The emulator receives the seed as a C int; without a range check, a value like 2**32-1 (e.g. from np.random.randint(2**32)) would silently wrap to - the -1 "time-dependent" sentinel, making an explicitly seeded env stochastic. + the -1 "stochastic" sentinel, making an explicitly seeded env stochastic. ''' if seed is None: return -1 @@ -389,14 +390,21 @@ def _resolve_seed(seed): return seed +# Episode seeds must stay distinct across forked workers (get_valid_actions' +# mp.Pool) and independent of random.seed(); SystemRandom draws from OS +# entropy with no process-local state, guaranteeing both. +_SYSTEM_RNG = random.SystemRandom() + + class FrotzEnv(): """ The Frotz Environment is a fast interface to Z-Machine games. :param story_file: Path to a Z-machine rom file (.z3/.z5/.z6/.z8). :param seed: Seed the random number generator used by the emulator. - Default: -1, i.e. the emulator's random number generator is - seeded with the current time, making episodes stochastic. + Default: None (equivalent to -1), i.e. a fresh random seed is + drawn for each episode, making episodes stochastic. The seed of the + current episode is available as :attr:`jericho.FrotzEnv.episode_seed`. :type story_file: path :type seed: int @@ -423,8 +431,9 @@ def load(self, story_file, seed=None): :param story_file: Path to a Z-machine rom file (.z3/.z5/.z6/.z8). :param seed: Seed the random number generator used by the emulator. - Default: -1, i.e. the emulator's random number generator is - seeded with the current time, making episodes stochastic. + Default: None (equivalent to -1), i.e. a fresh random seed is + drawn for each episode, making episodes stochastic. The seed of the + current episode is available as :attr:`jericho.FrotzEnv.episode_seed`. :type story_file: path :type seed: int ''' @@ -458,9 +467,25 @@ def load(self, story_file, seed=None): self._seed = _resolve_seed(seed) self._warned_implicit_seed = False self._episode_seed_implicit = not self._seed_is_explicit - self.frotz_lib.setup(self.story_file, self._seed, rom, len(rom)) + self.frotz_lib.setup(self.story_file, self._next_episode_seed(), rom, len(rom)) self.player_obj_num = self.frotz_lib.get_self_object_num() + def _next_episode_seed(self): + ''' + The concrete seed to hand to the emulator for the episode being started. + + The stochastic sentinel (-1) must never reach the emulator: it would + trigger the `time(0)` fallback in `os_random_seed`, whose one-second + resolution makes e.g. parallel unseeded envs play identical episodes. + Resolving it here to a drawn seed also keeps every episode reproducible + after the fact (see :attr:`jericho.FrotzEnv.episode_seed`). + ''' + if self._seed == -1: + self._episode_seed = _SYSTEM_RNG.getrandbits(31) + else: + self._episode_seed = self._seed + return self._episode_seed + def _maybe_warn_implicit_seed(self, stacklevel): ''' Warns (at most once per loaded game) when an episode is played without @@ -479,10 +504,11 @@ def _maybe_warn_implicit_seed(self, stacklevel): # the user gets a single exception, not one per reset()/step() forever. self._warned_implicit_seed = True msg = ("Since Jericho 4.0, the walkthrough seed ({}) of game '{}' is no longer used" - " by default, i.e. this episode is stochastic (time-dependent seed)." + " by default, i.e. this episode is stochastic (randomly drawn seed: {})." " Call reset(use_walkthrough_seed=True) to reproduce the walkthrough," " or make stochasticity explicit (e.g. FrotzEnv(rom, seed=-1) or env.seed(-1))" - " to silence this warning.").format(self.walkthrough_seed, self.story_file.decode()) + " to silence this warning.").format(self.walkthrough_seed, self.story_file.decode(), + self._episode_seed) warnings.warn(msg, ImplicitRandomSeedWarning, stacklevel=stacklevel) def seed(self, seed=None): @@ -490,9 +516,10 @@ def seed(self, seed=None): Changes seed used for the emulator's random number generator. :param seed: Seed the random number generator used by the emulator. - Default: -1, i.e. the emulator's random number generator is - seeded with the current time, making episodes stochastic. - :returns: The value of the seed. + Default: None (equivalent to -1), i.e. a fresh random seed is + drawn for each episode, making episodes stochastic. The seed of the + current episode is available as :attr:`jericho.FrotzEnv.episode_seed`. + :returns: The value of the seed (-1 stands for "draw one per episode"). .. note:: :meth:`jericho.FrotzEnv.reset()` must be called before the seed takes effect. @@ -503,8 +530,8 @@ def seed(self, seed=None): to reproduce a walkthrough. .. note:: Calling this method counts as an explicit seeding choice, even - without an argument (i.e. deliberately requesting a - time-dependent seed), so subsequent episodes do not raise + without an argument (i.e. deliberately requesting stochastic + episodes), so subsequent episodes do not raise :class:`jericho.ImplicitRandomSeedWarning`. ''' @@ -530,18 +557,39 @@ def walkthrough_seed(self): ''' return self.bindings.get('seed') + @property + def episode_seed(self): + ''' + The seed the emulator was seeded with at the start of the current episode. + + For a stochastic env this is the randomly drawn seed of the episode, so any + episode can be reproduced after the fact, e.g. with + `FrotzEnv(rom, seed=env.episode_seed)`. + + :returns: The current episode's seed. + + .. note:: Restoring a mid-episode state with + :meth:`jericho.FrotzEnv.set_state` does not update this value: + the restored state carries the RNG registers of the episode it + was captured from, not a seed. + ''' + return self._episode_seed + def reset(self, use_walkthrough_seed=False): ''' Resets the game. :param use_walkthrough_seed: Seed the emulator to reproduce the walkthrough. Default: `False`, i.e. use the seed set with - :meth:`jericho.FrotzEnv.seed` (a time-dependent + :meth:`jericho.FrotzEnv.seed` (a randomly drawn seed, unless one was explicitly provided). :type use_walkthrough_seed: bool :returns: A tuple containing the initial observation,\ - and a dictionary of info. + and a dictionary of info (including the `seed` used for this episode). :rtype: string, dictionary + :raises ValueError: If `use_walkthrough_seed=True` but no walkthrough seed + is known for this game. Check + `env.walkthrough_seed is None` to handle such games. .. note:: Using `use_walkthrough_seed=True` makes the game deterministic. As described in the Jericho paper, this is a *handicap* that @@ -553,24 +601,24 @@ def reset(self, use_walkthrough_seed=False): walkthrough seed persistent, use `env.seed(env.walkthrough_seed)`. ''' - seed = self._seed if use_walkthrough_seed: if self.walkthrough_seed is None: - msg = ("No walkthrough seed is known for game '{}'," - " using the environment's seed instead.").format(self.story_file.decode()) - warnings.warn(msg, UnsupportedGameWarning, stacklevel=2) - else: - seed = self.walkthrough_seed + raise ValueError( + "No walkthrough seed is known for game '{}'. Check" + " `env.walkthrough_seed is None` before requesting" + " use_walkthrough_seed=True.".format(self.story_file.decode())) + seed = self._episode_seed = self.walkthrough_seed + else: + seed = self._next_episode_seed() - episode_explicit = self._seed_is_explicit or (use_walkthrough_seed and self.walkthrough_seed is not None) - self._episode_seed_implicit = not episode_explicit + self._episode_seed_implicit = not (self._seed_is_explicit or use_walkthrough_seed) self._maybe_warn_implicit_seed(stacklevel=3) self.close() rom, _, _ = self._cache[self.story_file.decode()] obs_ini = self.frotz_lib.setup(self.story_file, seed, rom, len(rom)).decode('cp1252') score = self.frotz_lib.get_score() - return obs_ini, {'moves':self.get_moves(), 'score':score} + return obs_ini, {'moves':self.get_moves(), 'score':score, 'seed':seed} def step(self, action): ''' @@ -765,6 +813,9 @@ def copy(self): env._seed_is_explicit = self._seed_is_explicit env._warned_implicit_seed = self._warned_implicit_seed env._episode_seed_implicit = self._episode_seed_implicit + # set_state() below makes the fork replay the original's episode, so + # it must report the original's episode seed, not the one load() drew. + env._episode_seed = self._episode_seed env.set_state(state) return env diff --git a/tests/test_seed.py b/tests/test_seed.py index f29f90c6..d8e7859d 100644 --- a/tests/test_seed.py +++ b/tests/test_seed.py @@ -1,5 +1,4 @@ import os -import time import warnings from os.path import join as pjoin @@ -26,7 +25,7 @@ def _quiet_env(*args, **kwargs): return jericho.FrotzEnv(*args, **kwargs) -def test_default_seed_is_time_dependent(): +def test_default_seed_is_stochastic(): # By default, the walkthrough seed should *not* be used silently. env = _quiet_env(ROM) assert env._seed == -1 @@ -46,7 +45,7 @@ def test_seed_validation(): env = jericho.FrotzEnv(ROM, seed=42) # The emulator takes a C int; values that don't fit must not silently wrap. - # E.g. 2**32-1 would wrap to the -1 "time-dependent" sentinel, silently + # E.g. 2**32-1 would wrap to the -1 "stochastic" sentinel, silently # making an explicitly seeded env stochastic. for bad in (2**32 - 1, 2**31, -2**31 - 1): with pytest.raises(ValueError): @@ -82,10 +81,13 @@ def test_constructor_episode_uses_explicit_seed(): def test_constructor_episode_is_stochastic(): # ...and without a seed it must not fall back to the walkthrough seed. + # Creating both envs within the same second is deliberate: seeds must come + # from OS entropy rather than a clock, so that e.g. parallel workers + # spawned together still play distinct episodes. env1 = _quiet_env(ROM) - time.sleep(1.1) # The time-dependent seed has one-second resolution. env2 = _quiet_env(ROM) assert _rng_state(env1) != _rng_state(env2) + assert env1.episode_seed != env2.episode_seed def test_unseeded_resets_are_stochastic(): @@ -94,11 +96,35 @@ def test_unseeded_resets_are_stochastic(): warnings.simplefilter("ignore", jericho.ImplicitRandomSeedWarning) env.reset() rng1 = _rng_state(env) - time.sleep(1.1) - env.reset() + env.reset() # A reset within the same second must still differ. assert _rng_state(env) != rng1 +def test_episode_seed_reproduces_stochastic_episode(): + env = _quiet_env(ROM) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", jericho.ImplicitRandomSeedWarning) + obs, info = env.reset() + + # The drawn seed is surfaced both as a property and in the info dict... + assert info['seed'] == env.episode_seed + + # ...and replaying with it reproduces the episode exactly. + replay = jericho.FrotzEnv(ROM, seed=env.episode_seed) + assert _rng_state(replay) == _rng_state(env) + + # An explicitly seeded env reports its seed too. + env = jericho.FrotzEnv(ROM, seed=42) + assert env.episode_seed == 42 + obs, info = env.reset() + assert info['seed'] == 42 + + # A walkthrough-seeded episode reports the walkthrough seed. + env = jericho.FrotzEnv(ROM) + obs, info = env.reset(use_walkthrough_seed=True) + assert info['seed'] == env.walkthrough_seed == env.episode_seed + + def test_warning_when_using_implicit_random_seed(): # Constructing is silent; the warning fires when the first episode is # actually played without an explicit seeding choice... @@ -124,7 +150,7 @@ def test_warning_when_using_implicit_random_seed(): jericho.FrotzEnv(ROM).reset(use_walkthrough_seed=True) env = jericho.FrotzEnv(ROM) - env.seed() # Deliberate request for time-dependent randomness. + env.seed() # Deliberate request for stochastic episodes. env.reset() # No warning for games without a walkthrough seed. @@ -139,6 +165,7 @@ def test_copy_preserves_seed_bookkeeping(): # The fork replays the parent's episode faithfully... assert _rng_state(fork) == _rng_state(env) + assert fork.episode_seed == env.episode_seed # ...and keeps the parent's seed bookkeeping instead of silently becoming # "explicitly seeded" via the seed= constructor argument copy() uses. assert fork._seed_is_explicit == env._seed_is_explicit @@ -154,15 +181,17 @@ def test_copy_preserves_seed_bookkeeping(): def test_reset_with_walkthrough_seed_but_no_bindings(): + # The caller asked for a specific deterministic setup that cannot be + # honored; silently substituting another seed would be the same trap as + # silently applying one (#84), so this must raise instead. env = jericho.FrotzEnv(ROM_NO_BINDINGS, seed=42) - with pytest.warns(jericho.UnsupportedGameWarning, match="environment's seed"): - env.reset(use_walkthrough_seed=True) - # It falls back to the env's seed (not a time-dependent one). + env.reset() rng1 = _rng_state(env) - time.sleep(1.1) - with pytest.warns(jericho.UnsupportedGameWarning): + with pytest.raises(ValueError, match="walkthrough seed"): env.reset(use_walkthrough_seed=True) + # The failed reset must not have touched the current episode. assert _rng_state(env) == rng1 + env.step('look') # Still playable. def test_reset_with_walkthrough_seed_applies_the_walkthrough_seed(): @@ -211,6 +240,6 @@ def test_set_state_restores_rng_across_envs(): env.step(act) state = env.get_state() - worker = jericho.FrotzEnv(ROM, seed=-1) # Different time-dependent seed. + worker = jericho.FrotzEnv(ROM, seed=-1) # Different randomly drawn seed. worker.set_state(state) assert _rng_state(worker) == _rng_state(env)