diff --git a/doc/tutorials/advanced_tutorials/h_selector_prototype.ipynb b/doc/tutorials/advanced_tutorials/h_selector_prototype.ipynb index dd587eb5bb..0395f0e243 100644 --- a/doc/tutorials/advanced_tutorials/h_selector_prototype.ipynb +++ b/doc/tutorials/advanced_tutorials/h_selector_prototype.ipynb @@ -106,7 +106,7 @@ "(call/fold after checking then facing a bet) must still distinguish her own card, even\n", "though the tree has grown well past where that distinction was first established.\n", "\n", - "`alice_partition` is built once, tagged `.with_recall(\"Alice\")`, and reused for both of her\n", + "`alice_partition` is built once, tagged `.by_last_action(\"Alice\")`, and reused for both of her\n", "decisions -- the tag makes `.plays` automatically fold her own last action into the group\n", "key from her second decision onward, with no separate re-derivation step. `bob_partition`\n", "never needs the tag: his two uses are his *one* decision instantiated on two mutually\n", @@ -132,7 +132,7 @@ } }, "outputs": [], - "source": "CARD_VALUE = {\"J\": 0, \"Q\": 1, \"K\": 2}\ncards = list(CARD_VALUE)\n\ng = gbt.ExtensiveGame(players=[\"Alice\", \"Bob\"], title=\"Kuhn poker\")\ng.append_event(H.path(), dict.fromkeys(cards, gbt.Rational(1, 3)))\nfor c in cards:\n remaining = [x for x in cards if x != c]\n g.append_event(H.path(c), dict.fromkeys(remaining, gbt.Rational(1, 2)))\n\nalice_partition = H.path(...).by(lambda h: h[0].action).with_recall(\"Alice\")\ng.append_move(alice_partition.plays, \"Alice\", [\"Check\", \"Bet\"])\n\nbob_partition = H.path(..., ...).by(lambda h: h[1].action)\ng.append_move(bob_partition.plays.after(\"Check\"), \"Bob\", [\"Check\", \"Bet\"])\n\ng.append_move(alice_partition.plays.after(\"Check\", \"Bet\"), \"Alice\", [\"Fold\", \"Call\"])\ng.append_move(bob_partition.plays.after(\"Bet\"), \"Bob\", [\"Fold\", \"Call\"])\n\ndraw(g)\nprint(\"is_perfect_recall:\", g.is_perfect_recall)" + "source": "CARD_VALUE = {\"J\": 0, \"Q\": 1, \"K\": 2}\ncards = list(CARD_VALUE)\n\ng = gbt.ExtensiveGame(players=[\"Alice\", \"Bob\"], title=\"Kuhn poker\")\ng.append_event(H.path(), dict.fromkeys(cards, gbt.Rational(1, 3)))\nfor c in cards:\n remaining = [x for x in cards if x != c]\n g.append_event(H.path(c), dict.fromkeys(remaining, gbt.Rational(1, 2)))\n\nalice_partition = H.path(...).by(lambda h: h[0].action).by_last_action(\"Alice\")\ng.append_move(alice_partition.plays, \"Alice\", [\"Check\", \"Bet\"])\n\nbob_partition = H.path(..., ...).by(lambda h: h[1].action)\ng.append_move(bob_partition.plays.after(\"Check\"), \"Bob\", [\"Check\", \"Bet\"])\n\ng.append_move(alice_partition.plays.after(\"Check\", \"Bet\"), \"Alice\", [\"Fold\", \"Call\"])\ng.append_move(bob_partition.plays.after(\"Bet\"), \"Bob\", [\"Fold\", \"Call\"])\n\ndraw(g)\nprint(\"is_perfect_recall:\", g.is_perfect_recall)" }, { "cell_type": "code", @@ -160,7 +160,7 @@ "`contrib/games/bayes2a.efg` in the repository. Both players privately learn a type, then\n", "move simultaneously each round; each round's actions become public before the next round.\n", "\n", - "Unlike Kuhn poker, this game needs neither `.with_recall` nor `append_infoset` -- every\n", + "Unlike Kuhn poker, this game needs neither `.by_last_action` nor `append_infoset` -- every\n", "player's decision falls at a fixed, predictable position in the history across every\n", "branch, so plain positional indexing on the augmented history object is all the grouping\n", "needs. This is deliberately included as a contrast case: `H`'s dedicated recall machinery\n", diff --git a/src/pygambit/gametree.pxi b/src/pygambit/gametree.pxi index 91f77e1254..cf1efd0ed8 100644 --- a/src/pygambit/gametree.pxi +++ b/src/pygambit/gametree.pxi @@ -441,9 +441,9 @@ class ExtensiveGame(Game): Applies `grouped`'s initial partition (`base`/`key`), then its `post_ops` in order, each one per-group -- expanding/filtering each group's own members independently, leaving the key untouched, except - that a `.plays` step refines the key by `recall_player`'s last action - at that point, if `with_recall` set one (see `GroupedSelector`'s - docstring for why). + that a `.plays` step refines the key by `last_action_player`'s last + action at that point, if `.by_last_action` set one (see + `GroupedSelector`'s docstring for why). """ result: dict = {} for node in self._get_nodes(grouped.base): @@ -457,11 +457,13 @@ class ExtensiveGame(Game): expanded = [ play for node in nodes for play in cython.cast(Node, node)._plays() ] - if grouped.recall_player is None: + if grouped.last_action_player is None: next_result[key] = expanded else: for play in expanded: - refined_key = (key, _last_action(play, grouped.recall_player)) + refined_key = ( + key, _last_action_key(play, grouped.last_action_player) + ) next_result.setdefault(refined_key, []).append(play) continue if isinstance(op, _AfterStep): diff --git a/src/pygambit/hsel.pxi b/src/pygambit/hsel.pxi index b590b6503d..016c4e5290 100644 --- a/src/pygambit/hsel.pxi +++ b/src/pygambit/hsel.pxi @@ -183,15 +183,25 @@ class GroupedSelector: chain onto a plain `Selector`, but apply per-group: each group's own members are expanded/filtered independently, and the group's key is left untouched -- expanding past a decision point doesn't retroactively change - what a group was keyed by. `.with_recall(player)` is the one exception: - once set, every subsequent `.plays` on this selector *also* refines each - group's key by folding in `player`'s last action at that point, so a - partition built for one decision stays a valid recall-respecting - partition when reused for a later one, without the caller needing to - re-derive or manually re-key it. Scoped to `.plays` specifically for now - (not every expand-style op) -- narrower than the full "any expand-style - step" idea from the design notes, not yet stress-tested against a shape - that would need more. + what a group was keyed by. `.by_last_action(player)` is the one + exception: once set, every subsequent `.plays` on this selector *also* + refines each group's key by folding in `player`'s last action at that + point -- keyed on the `(infoset, label)` pair identifying that action, + not the label alone, so two infosets that happen to share an action + label stay distinct groups -- so a partition built for one decision + stays a valid recall-respecting partition when reused for a later one, + without the caller needing to re-derive or manually re-key it. Scoped to + `.plays` specifically for now (not every expand-style op) -- narrower + than the full "any expand-style step" idea from the design notes, not + yet stress-tested against a shape that would need more. + + The refinement is relative to this selector chain, not a general recall + guarantee: it only folds in the action taken at the `.plays` step + immediately before it. A `.plays` that jumps past more than one of + `player`'s decisions, or a base partition that already pools together + histories differing in an earlier one of `player`'s actions, will + silently under-refine -- `.by_last_action` cannot recover information + the selector chain already discarded. .. versionadded:: 17.0.0 """ @@ -201,27 +211,29 @@ class GroupedSelector: base: Selector, key: typing.Callable, post_ops: tuple = (), - recall_player: str = None, + last_action_player: str = None, ) -> None: self.base = base self.key = key self.post_ops = post_ops - self.recall_player = recall_player + self.last_action_player = last_action_player def __repr__(self) -> str: return ( f"GroupedSelector(base={self.base!r}, key={self.key!r}, " - f"post_ops={self.post_ops!r}, recall_player={self.recall_player!r})" + f"post_ops={self.post_ops!r}, last_action_player={self.last_action_player!r})" ) def _extend(self, op) -> GroupedSelector: - return GroupedSelector(self.base, self.key, self.post_ops + (op,), self.recall_player) + return GroupedSelector( + self.base, self.key, self.post_ops + (op,), self.last_action_player + ) @property def plays(self) -> GroupedSelector: """The current terminal frontier of each group, independently -- see the class docstring for how this interacts with - `.with_recall(player)`.""" + `.by_last_action(player)`.""" return self._extend(_PlaysStep()) def after(self, *labels: str) -> GroupedSelector: @@ -229,7 +241,7 @@ class GroupedSelector: are exactly `labels`, whatever came before them.""" return self._extend(_AfterStep(labels)) - def with_recall(self, player: str) -> GroupedSelector: + def by_last_action(self, player: str) -> GroupedSelector: """From here on, every `.plays` on this selector also refines each group's key by folding in `player`'s last action at that point -- see the class docstring.""" @@ -296,11 +308,12 @@ def _canonical_history_cached(node: Node, cache: dict) -> History: @cython.cfunc -def _node_last_action(node: c_GameNode, player: str) -> object: - """The label of the last action `player` took on the path to `node` - (a raw handle, not a `Node`), wherever it fell -- `None` if `player` - hasn't acted yet. The `c_GameNode`-only core of `_last_action`, used - directly by `HistoryView`, which must never hold a `Node`. +def _node_last_acted_child(node: c_GameNode, player: str) -> c_GameNode: + """Internal: the child (a raw handle, not a `Node`) reached by `player`'s + last action on the path to `node` -- i.e. the node whose parent is where + `player` acted -- or a null handle if `player` hasn't acted yet. Shared + by `_node_last_action` (the action's label) and `_node_last_action_key` + (the `(infoset, label)` pair `.by_last_action(player)` keys on). Raises ------ @@ -321,17 +334,68 @@ def _node_last_action(node: c_GameNode, player: str) -> object: player_handle != cython.cast(c_GamePlayer, NULL) and player_handle.deref().GetLabel().decode("utf-8") == player ): - return current.deref().GetPriorAction().deref().GetLabel().decode("utf-8") + return current current = parent parent = current.deref().GetParent() + return cython.cast(c_GameNode, NULL) + + +@cython.cfunc +def _node_last_action(node: c_GameNode, player: str) -> object: + """The label of the last action `player` took on the path to `node` + (a raw handle, not a `Node`), wherever it fell -- `None` if `player` + hasn't acted yet. The `c_GameNode`-only core of `_last_action`, used + directly by `HistoryView`, which must never hold a `Node`. + + Raises + ------ + KeyError + If no player in the game has label `player`. + TypeError + If `player` is not a `str`. + ValueError + If `player` is an empty string or all spaces. + """ + child: c_GameNode = _node_last_acted_child(node, player) + if child != cython.cast(c_GameNode, NULL): + return child.deref().GetPriorAction().deref().GetLabel().decode("utf-8") + return None + + +@cython.cfunc +def _node_last_action_key(node: c_GameNode, player: str) -> object: + """The `(infoset, label)` pair identifying `player`'s last action on the + path to `node` (a raw handle, not a `Node`), wherever it fell -- `None` + if `player` hasn't acted yet. `infoset` is the canonical `History` of the + infoset at which the action was taken (see `_canonical_history`, + matching `HistoryTransition.state`), so two infosets that happen to + share an action label are still told apart. Used by + `.by_last_action(player)`'s evaluation, via `_last_action_key`. + + Raises + ------ + KeyError + If no player in the game has label `player`. + TypeError + If `player` is not a `str`. + ValueError + If `player` is an empty string or all spaces. + """ + child: c_GameNode = _node_last_acted_child(node, player) + if child != cython.cast(c_GameNode, NULL): + return ( + _canonical_history(Node.wrap(child.deref().GetParent())), + child.deref().GetPriorAction().deref().GetLabel().decode("utf-8"), + ) return None -def _last_action(node: Node, player: str) -> str | None: - """The label of the last action `player` took on the path to `node`, - wherever it fell -- `None` if `player` hasn't acted yet. Used by - `.with_recall(player)`'s evaluation, which still works in terms of `Node`.""" - return _node_last_action(cython.cast(Node, node).node, player) +def _last_action_key(node: Node, player: str) -> tuple | None: + """The `(infoset, label)` pair identifying `player`'s last action on the + path to `node`, wherever it fell -- `None` if `player` hasn't acted yet. + Used by `.by_last_action(player)`'s evaluation, which still works in + terms of `Node`.""" + return _node_last_action_key(cython.cast(Node, node).node, player) @cython.cclass diff --git a/tests/test_hsel.py b/tests/test_hsel.py index 95023e9ef0..e4a491c691 100644 --- a/tests/test_hsel.py +++ b/tests/test_hsel.py @@ -156,24 +156,46 @@ def key(h): game._get_groups(gbt.H.plays.by(key)) -def test_with_recall_raises_on_invalid_player(): +def test_by_last_action_raises_on_invalid_player(): game = gbt.ExtensiveGame(players=["A", "B"]) game.append_move(gbt.H.path(), "A", ["U", "D"]) game.append_move(gbt.H.plays, "B", ["x", "y"]) - grouped = gbt.H.path().by(lambda h: None).with_recall("NoSuchPlayer").plays + grouped = gbt.H.path().by(lambda h: None).by_last_action("NoSuchPlayer").plays with pytest.raises(KeyError): game._get_groups(grouped) -def test_with_recall_refines_key_by_last_action(): +def test_by_last_action_refines_key_by_last_action(): game = gbt.ExtensiveGame(players=["A", "B"]) game.append_move(gbt.H.path(), "A", ["U", "D"]) game.append_move(gbt.H.plays, "B", ["x", "y"]) - grouped = gbt.H.path().by(lambda h: None).with_recall("A").plays + grouped = gbt.H.path().by(lambda h: None).by_last_action("A").plays groups = game._get_groups(grouped) - assert {k: [h.actions for h in v] for k, v in groups.items()} == { - (None, "U"): [("U", "x"), ("U", "y")], - (None, "D"): [("D", "x"), ("D", "y")], + keyed_by_action = {k[1]: [h.actions for h in v] for k, v in groups.items()} + assert {label: histories for (_, label), histories in keyed_by_action.items()} == { + "U": [("U", "x"), ("U", "y")], + "D": [("D", "x"), ("D", "y")], + } + + +def test_by_last_action_distinguishes_infosets_sharing_a_label(): + # A acts at two distinct infosets (one per branch of the chance move), both + # offering an action labelled "U". Grouping only by the label would wrongly + # merge these into one group, breaking perfect recall by construction -- + # `.by_last_action` must key on the (infoset, label) pair instead. + game = gbt.ExtensiveGame(players=["A", "B"]) + game.append_event(gbt.H.path(), {"L": gbt.Rational(1, 2), "R": gbt.Rational(1, 2)}) + game.append_move(gbt.H.path("L"), "A", ["U", "D"]) + game.append_move(gbt.H.path("R"), "A", ["U", "D"]) + + grouped = gbt.H.path(...).by(lambda h: None).by_last_action("A").plays + groups = game._get_groups(grouped) + assert len(groups) == 4 + assert {frozenset(h.actions for h in v) for v in groups.values()} == { + frozenset({("L", "U")}), + frozenset({("L", "D")}), + frozenset({("R", "U")}), + frozenset({("R", "D")}), }