-
Notifications
You must be signed in to change notification settings - Fork 47
Make the walkthrough seed opt-in: unseeded FrotzEnv is stochastic (v4.0) #88
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,6 +17,7 @@ | |
| import os | ||
| import shutil | ||
| import tempfile | ||
| import operator | ||
| import warnings | ||
| import hashlib | ||
|
|
||
|
|
@@ -368,17 +369,42 @@ class TruncatedInputActionWarning(UserWarning): | |
| pass | ||
|
|
||
|
|
||
| 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. | ||
|
|
||
| :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 +423,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 | ||
| ''' | ||
|
|
@@ -425,39 +451,124 @@ 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. | ||
|
|
||
| :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. | ||
|
|
||
| .. 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`. | ||
|
|
||
| ''' | ||
| seed = seed or self.bindings.get('seed', -1) | ||
| self._seed = seed | ||
| return seed | ||
| self._seed_is_explicit = True | ||
| self._seed = _resolve_seed(seed) | ||
| return self._seed | ||
|
|
||
| def reset(self): | ||
| @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) # Applies the walkthrough seed to this episode only. | ||
|
|
||
| ''' | ||
| return self.bindings.get('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 | ||
| 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. | ||
|
|
||
| .. 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 the environment's seed instead.").format(self.story_file.decode()) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Open question: when the caller explicitly requests
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a great point. I agree, it is better to raise when possible. I'm not a big fan of warning messages since most of the time, they can easily go undetected. So, +1 for raising a ValueError.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, done!
|
||
| warnings.warn(msg, UnsupportedGameWarning, stacklevel=2) | ||
| else: | ||
| seed = self.walkthrough_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._maybe_warn_implicit_seed(stacklevel=3) | ||
|
|
||
| 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} | ||
|
|
||
|
|
@@ -476,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] | ||
|
|
@@ -579,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 | ||
|
|
||
|
|
@@ -607,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) | ||
|
|
@@ -636,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 | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| __version__ = '3.3.1' | ||
| __version__ = '4.0.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open question: the
-1sentinel makes the emulator seed itself fromtime(0) & 0x7fff(seeos_random_seedindumb_init.c), i.e. one-second resolution and a 15-bit space. Unseeded envs created in the same second play identical episodes, which particularly affects parallel and vectorized runs, arguably the main audience for stochastic-by-default. I've documented the caveat in the README for now, but a stronger fix would be resolvingseed=Noneon the Python side (e.g. fromos.urandom) into a concrete random seed and passing that down, which also makes the episode's seed inspectable after the fact. That changes the meaning of the-1sentinel though, so I left it out of this PR. Thoughts?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great catch. I like your proposed idea, dealing with it on the Python does make things more reproducible, we should also probably display which seed was obtained from Python's RNG and used to seed the z-machine. If you can implement this, that would be awesome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, can you PTAL?
Unseeded envs now draw a new 31-bit seed per episode on the Python side, so
-1never reaches the emulator and same-second envs no longer collide. The selected seed is surfaced in three places:FrotzEnv.episode_seedproperty,seedkey inreset()'s info dict,ImplicitRandomSeedWarningmessageThe goal is to make any episode reproducible after the fact with
FrotzEnv(rom, seed=env.episode_seed).There's one deviation from the "Python's RNG" suggestion in the comment above: seeds are drawn from
random.SystemRandom(backed by OS entropy) rather than the globalrandommodule.Two reasons:
get_valid_actions(use_parallel=True)forks workers viamp.Pool, and forked processes inherit the global Mersenne state: every worker would draw the same "random" seed, reintroducing the exact collision this change fixes.random.seed(0)from silently turning "stochastic" envs deterministic, which felt like the same "unexpected determinism" pitfall as Silent fixed-seed default prevents independent runs for most games. Consider adding a warning? #84.The cost is that unseeded episodes aren't reproducible via global seeding. I think that's acceptable, given what we gain, and ultimately, that's what
episode_seedis for. However, let me know if you disagree or you'd like to take this in a different direction.I kept
-1as the documented "explicitly stochastic" sentinel (it now means "draw a random seed per episode" rather than "let the emulator use the clock"), since the warning message and docs already point users at it. I tried to update all comments accordingly, but please let me know if I missed any.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's reasonable. Thank you again for the PR.