From 3b7ffe982f1781f5250a7d55a4610696eb8c63c7 Mon Sep 17 00:00:00 2001 From: drdkad Date: Wed, 3 Dec 2025 21:03:18 +0000 Subject: [PATCH 01/18] Refactor the legacy subgame root finder using DSU (Disjoint Set Union) of generated components --- src/games/game.h | 3 + src/games/gameagg.h | 1 + src/games/gamebagg.h | 1 + src/games/gametable.h | 1 + src/games/gametree.cc | 212 ++++++++++++++++++++++++++++++++++++++++ src/games/gametree.h | 9 ++ src/pygambit/gambit.pxd | 1 + src/pygambit/game.pxi | 4 + tests/test_node.py | 92 +++++++++++------ 9 files changed, 296 insertions(+), 28 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index 7cb3a315f..57f92b17f 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -432,6 +432,7 @@ class GameNodeRep : public std::enable_shared_from_this { friend class GameInfosetRep; friend class GamePlayerRep; friend class PureBehaviorProfile; + friend class SubgameRootFinder; template friend class MixedBehaviorProfile; bool m_valid{true}; @@ -836,6 +837,8 @@ class GameRep : public std::enable_shared_from_this { } return false; } + /// + virtual GameNode GetSubgameRoot(GameInfoset) const = 0; //@} /// @name Writing data files diff --git a/src/games/gameagg.h b/src/games/gameagg.h index bd12f7da7..e48f782e0 100644 --- a/src/games/gameagg.h +++ b/src/games/gameagg.h @@ -81,6 +81,7 @@ class GameAGGRep : public GameRep { bool IsTree() const override { return false; } bool IsAgg() const override { return true; } bool IsPerfectRecall() const override { return true; } + GameNode GetSubgameRoot(GameInfoset) const override { return {nullptr}; } bool IsConstSum() const override; /// Returns the smallest payoff to any player in any outcome of the game Rational GetMinPayoff() const override { return Rational(aggPtr->getMinPayoff()); } diff --git a/src/games/gamebagg.h b/src/games/gamebagg.h index 3918a58d2..f976dca38 100644 --- a/src/games/gamebagg.h +++ b/src/games/gamebagg.h @@ -88,6 +88,7 @@ class GameBAGGRep : public GameRep { bool IsTree() const override { return false; } virtual bool IsBagg() const { return true; } bool IsPerfectRecall() const override { return true; } + GameNode GetSubgameRoot(GameInfoset) const override { return {nullptr}; } bool IsConstSum() const override { throw UndefinedException(); } /// Returns the smallest payoff to any player in any outcome of the game Rational GetMinPayoff() const override { return Rational(baggPtr->getMinPayoff()); } diff --git a/src/games/gametable.h b/src/games/gametable.h index 3aab1dd38..edc25687e 100644 --- a/src/games/gametable.h +++ b/src/games/gametable.h @@ -64,6 +64,7 @@ class GameTableRep : public GameExplicitRep { Rational GetPlayerMaxPayoff(const GamePlayer &) const override; bool IsPerfectRecall() const override { return true; } + GameNode GetSubgameRoot(GameInfoset) const override { return {nullptr}; } //@} /// @name Dimensions of the game diff --git a/src/games/gametree.cc b/src/games/gametree.cc index c630c50a2..cce9a0e50 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -1093,6 +1094,217 @@ void GameTreeRep::BuildUnreachableNodes() const } } +//------------------------------------------------------------------------ +// Subgame Root Finder +//------------------------------------------------------------------------ + +namespace { // Anonymous namespace for internal implementation + +// Comparator for priority queue (ancestors have lower priority than descendants) +struct NodeCmp { + bool operator()(const GameNodeRep *a, const GameNodeRep *b) const + { + return a->GetNumber() < b->GetNumber(); + } +}; + +// Scratch data structure for subgame root finding algorithm +struct SubgameScratchData { + // DSU structures + std::vector dsu_parent; + std::vector leader_subgame_map; + + // ID mapping + std::map infoset_to_id; + + // Visited tracking + std::vector infoset_visited; + std::vector node_visited_token; + int current_token = 0; +}; + +// DSU Find with path compression +int FindSet(SubgameScratchData &p_data, int p_i) +{ + if (p_data.dsu_parent[p_i] != p_i) { + p_data.dsu_parent[p_i] = FindSet(p_data, p_data.dsu_parent[p_i]); + } + return p_data.dsu_parent[p_i]; +} + +// DSU Union - merge set j into set i, tracking best subgame root +void UnionSets(SubgameScratchData &p_data, int p_i, int p_j, GameNodeRep *p_current_node) +{ + const int leader_i = FindSet(p_data, p_i); + const int leader_j = FindSet(p_data, p_j); + + if (leader_i == leader_j) { + p_data.leader_subgame_map[leader_i] = p_current_node; + return; + } + + // Prefer j's root if it exists, otherwise use current node + auto *best_candidate = + p_data.leader_subgame_map[leader_j] ? p_data.leader_subgame_map[leader_j] : p_current_node; + + p_data.dsu_parent[leader_j] = leader_i; + p_data.leader_subgame_map[leader_i] = best_candidate; +} + +// Generate a single component starting from a given node +void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) +{ + const int start_id = p_data.infoset_to_id[p_start_node->GetInfoset().get()]; + p_data.current_token++; + + std::priority_queue, NodeCmp> local_frontier; + + local_frontier.push(p_start_node); + p_data.node_visited_token[p_start_node->GetNumber()] = p_data.current_token; + + while (!local_frontier.empty()) { + auto *curr = local_frontier.top(); + local_frontier.pop(); + + const int curr_id = p_data.infoset_to_id[curr->GetInfoset().get()]; + + UnionSets(p_data, start_id, curr_id, curr); + + // External collision: current node belongs to a previously generated component + if (p_data.infoset_visited[curr_id]) { + const int leader = FindSet(p_data, curr_id); + auto *candidate_root = p_data.leader_subgame_map[leader]; + + if (p_data.node_visited_token[candidate_root->GetNumber()] != p_data.current_token) { + local_frontier.push(candidate_root); + p_data.node_visited_token[candidate_root->GetNumber()] = p_data.current_token; + } + } + else { + p_data.infoset_visited[curr_id] = true; + } + + // adds other members of the corresponding infoset to the frontier + for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { + auto *member = member_sp.get(); + if (p_data.node_visited_token[member->GetNumber()] != p_data.current_token) { + local_frontier.push(member); + p_data.node_visited_token[member->GetNumber()] = p_data.current_token; + } + } + // if the frontier is not empty and the parent exists, add it to the frontier + if (!local_frontier.empty()) { + if (auto parent_sp = curr->GetParent()) { + auto *parent = parent_sp.get(); + if (p_data.node_visited_token[parent->GetNumber()] != p_data.current_token) { + local_frontier.push(parent); + p_data.node_visited_token[parent->GetNumber()] = p_data.current_token; + } + } + } + } +} + +// Main algorithm: find subgame roots for all infosets +std::map FindSubgameRoots(const Game &p_game) +{ + if (p_game->GetRoot()->IsTerminal()) { + return {}; + } + SubgameScratchData data; + + // Initialize infoset ID mapping using iterators + int next_id = 0; + + // Map Chance player's infosets + for (const auto &infoset : p_game->GetChance()->GetInfosets()) { + data.infoset_to_id[infoset.get()] = next_id++; + } + + // Map individual player's infosets + for (const auto &player : p_game->GetPlayers()) { + for (const auto &infoset : player->GetInfosets()) { + data.infoset_to_id[infoset.get()] = next_id++; + } + } + + // Initialize DSU structures + const int total_infosets = next_id; + data.dsu_parent.resize(total_infosets); + std::iota(data.dsu_parent.begin(), data.dsu_parent.end(), 0); + data.leader_subgame_map.assign(total_infosets, nullptr); + + // Initialize tracking + data.infoset_visited.assign(total_infosets, false); + data.node_visited_token.assign(p_game->NumNodes() + 1, 0); + + // Build global frontier (priority queue of nodes to process) + std::priority_queue, NodeCmp> global_frontier; + + data.current_token++; + + // Add parents of terminal nodes to the global frontier + for (const auto &node : p_game->GetNodes()) { + if (node->IsTerminal()) { + auto *parent = node->GetParent().get(); + if (data.node_visited_token[parent->GetNumber()] != data.current_token) { + global_frontier.push(parent); + data.node_visited_token[parent->GetNumber()] = data.current_token; + } + } + } + + // Process components in descending node order + while (!global_frontier.empty()) { + auto *node = global_frontier.top(); + global_frontier.pop(); + + const int id = data.infoset_to_id[node->GetInfoset().get()]; + + if (data.infoset_visited[id]) { + continue; + } + + GenerateComponent(data, node); + + auto *component_top_node = data.leader_subgame_map[FindSet(data, id)]; + + if (auto parent_sp = component_top_node->GetParent()) { + auto *parent = parent_sp.get(); + if (data.node_visited_token[parent->GetNumber()] == 0) { + global_frontier.push(parent); + data.node_visited_token[parent->GetNumber()] = data.current_token; + } + } + } + + std::map result; + + auto collect_results = [&](const GamePlayer &p_player) { + for (const auto &infoset : p_player->GetInfosets()) { + const int id = data.infoset_to_id[infoset.get()]; + result[infoset.get()] = data.leader_subgame_map[FindSet(data, id)]; + } + }; + + collect_results(p_game->GetChance()); + for (const auto &player : p_game->GetPlayers()) { + collect_results(player); + } + + return result; +} + +} // end anonymous namespace + +void GameTreeRep::BuildSubgameRoots() +{ + if (!m_computedValues) { + BuildComputedValues(); + } + m_infosetSubgameRoot = FindSubgameRoots(shared_from_this()); +} + //------------------------------------------------------------------------ // GameTreeRep: Writing data files //------------------------------------------------------------------------ diff --git a/src/games/gametree.h b/src/games/gametree.h index 9c96939f1..3754aa9bc 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -48,6 +48,7 @@ class GameTreeRep : public GameExplicitRep { mutable std::shared_ptr m_ownPriorActionInfo; mutable std::unique_ptr> m_unreachableNodes; mutable std::set m_absentMindedInfosets; + std::map m_infosetSubgameRoot; /// @name Private auxiliary functions //@{ @@ -88,6 +89,13 @@ class GameTreeRep : public GameExplicitRep { /// Returns the largest payoff to the player in any play of the game Rational GetPlayerMaxPayoff(const GamePlayer &) const override; bool IsAbsentMinded(const GameInfoset &p_infoset) const override; + GameNode GetSubgameRoot(GameInfoset infoset) const override + { + if (m_infosetSubgameRoot.empty()) { + const_cast(this)->BuildSubgameRoots(); + } + return {m_infosetSubgameRoot.at(infoset.get())->shared_from_this()}; + } //@} /// @name Players @@ -174,6 +182,7 @@ class GameTreeRep : public GameExplicitRep { std::vector BuildConsistentPlaysRecursiveImpl(GameNodeRep *node); void BuildOwnPriorActions() const; void BuildUnreachableNodes() const; + void BuildSubgameRoots(); }; template class TreeMixedStrategyProfileRep : public MixedStrategyProfileRep { diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index 0234f7920..4b4c00669 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -301,6 +301,7 @@ cdef extern from "games/game.h": stdvector[c_GameNode] GetPlays(c_GameAction) except + bool IsPerfectRecall() except + bool IsAbsentMinded(c_GameInfoset) except + + c_GameNode GetSubgameRoot(c_GameInfoset) except + c_GameInfoset AppendMove(c_GameNode, c_GamePlayer, int) except +ValueError c_GameInfoset AppendMove(c_GameNode, c_GameInfoset) except +ValueError diff --git a/src/pygambit/game.pxi b/src/pygambit/game.pxi index 1ed7fff42..1f77230a2 100644 --- a/src/pygambit/game.pxi +++ b/src/pygambit/game.pxi @@ -756,6 +756,10 @@ class Game: """ return self.game.deref().IsPerfectRecall() + def subgame_root(self, infoset: typing.Union[Infoset, str]) -> Node: + infoset = self._resolve_infoset(infoset, "subgame_root") + return Node.wrap(self.game.deref().GetSubgameRoot(cython.cast(Infoset, infoset).infoset)) + @property def min_payoff(self) -> decimal.Decimal | Rational: """The minimum payoff to any player in any play of the game. diff --git a/tests/test_node.py b/tests/test_node.py index 012e9ce0a..62309134a 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -93,32 +93,6 @@ def test_is_successor_of(): game.root.is_successor_of(game.players[0]) -@pytest.mark.parametrize("game, expected_result", [ - # Games without Absent-Mindedness for which the legacy method is known to be correct. - (games.read_from_file("wichardt.efg"), {0}), - (games.read_from_file("e02.efg"), {0, 2, 4}), - (games.read_from_file("subgames.efg"), {0, 1, 4, 7, 11, 13, 34}), - - pytest.param( - games.read_from_file("AM-driver-subgame.efg"), - {0, 3}, # The correct set of subgame roots - marks=pytest.mark.xfail( - reason="Current method does not detect roots of proper subgames " - "that are members of AM-infosets." - ) - ), -]) -def test_legacy_is_subgame_root_set(game: gbt.Game, expected_result: set): - """ - Tests the legacy `node.is_subgame_root` against an expected set of nodes. - Includes both passing cases and games with Absent-Mindedness where it is expected to fail. - """ - list_nodes = list(game.nodes) - expected_roots = {list_nodes[i] for i in expected_result} - legacy_roots = {node for node in game.nodes if node.is_subgame_root} - assert legacy_roots == expected_roots - - def _get_path_of_action_labels(node: gbt.Node) -> list[str]: """ Computes the path of action labels from the root to the given node. @@ -220,6 +194,68 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): assert actual_node_data == expected_node_data +# ============================================================================== +# New Test Suite for the Subgame Root Checker +# ============================================================================== + +@pytest.mark.parametrize("game, expected_result", [ + # Games without Absent-Mindedness for which the legacy method is known to be correct. + (games.read_from_file("wichardt.efg"), {0}), + (games.read_from_file("e02.efg"), {0, 2, 4}), + (games.read_from_file("merge.efg"), {0, 2}), + (games.read_from_file("sg.efg"), {0, 1, 4, 7, 11, 13, 34}), + (games.read_from_file("subgame-8-roots.efg"), {0, 1, 2, 4, 5, 16, 27, 30}), + (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], {0, 2, 4, 6, 8}), + (gbt.Game.new_tree(), {}), + + # Games with Absent-Mindedness where the legacy method is known to fail. + pytest.param( + games.read_from_file("AM-subgames.efg"), + {0, 2, 5, 8}, # The correct set of roots + marks=pytest.mark.xfail( + reason="Legacy method does not detect subgame roots that are members of AM-infosets." + ) + ), +]) +def test_legacy_is_subgame_root_set(game: gbt.Game, expected_result: set): + """ + Tests the legacy `node.is_subgame_root` against an expected set of nodes. + Includes both passing cases and games with Absent-Mindedness where it is expected to fail. + """ + list_nodes = list(game.nodes) + expected_roots = {list_nodes[i] for i in expected_result} + legacy_roots = {node for node in game.nodes if node.is_subgame_root} + assert legacy_roots == expected_roots + + +# NEW subgame finder algorithm +@pytest.mark.parametrize("game, expected_result", [ + # Games without Absent-Mindedness. + (games.read_from_file("e02.efg"), {0, 2, 4}), + (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], {0, 2, 4, 6, 8}), + (games.read_from_file("wichardt.efg"), {0}), + (games.read_from_file("merge.efg"), {0, 2}), + (games.read_from_file("sg.efg"), {0, 1, 4, 7, 11, 13, 34}), + (games.read_from_file("subgame-8-roots.efg"), {0, 1, 2, 4, 5, 16, 27, 30}), + (gbt.Game.new_tree(), {}), + + # Games with Absent-Mindedness, which the new method handles correctly. + (games.read_from_file("AM-subgames.efg"), {0, 2, 5, 8}), +]) +def test_new_subgame_root_method_set(game: gbt.Game, expected_result: set): + """ + Tests the new `game.subgame_root()` method against an expected set of nodes. + This method is expected to pass all cases, including those with Absent-Mindedness. + """ + list_nodes = list(game.nodes) + expected_roots = {list_nodes[i] for i in expected_result} + + actual_roots = {game.subgame_root(infoset) + for infoset in game.infosets if not infoset.is_chance} + + assert actual_roots == expected_roots + + @pytest.mark.parametrize("game_file, expected_unreachable_paths", [ # Games without absent-mindedness, where all nodes are reachable ("e02.efg", []), @@ -229,13 +265,13 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): # An absent-minded driver game with an unreachable terminal node ( "AM-driver-one-infoset.efg", - [["T", "S"]] + [["S", "T"]] ), # An absent-minded driver game with an unreachable subtree ( "AM-driver-subgame.efg", - [["T", "S"], ["r", "T", "S"], ["l", "T", "S"]] + [["S", "T"], ["S", "T", "r"], ["S", "T", "l"]] ), ]) def test_is_strategy_reachable(game_file: str, expected_unreachable_paths: list[list[str]]): From eafc248c80ec1ec4b3d06624b5b6dc7322e6ed39 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 4 Dec 2025 11:05:59 +0000 Subject: [PATCH 02/18] Add tests following the new path style for nodes and new .efg files --- src/games/gametree.cc | 21 ++- tests/test_games/AM-subgames.efg | 14 ++ .../subgame-roots-finder-multiple-merges.efg | 55 +++++++ .../subgame-roots-finder-one-merge.efg | 28 ++++ ...oots-finder-small-subgames-and-merges.efg} | 24 ++- tests/test_infosets.py | 12 +- tests/test_node.py | 145 ++++++++++++++---- 7 files changed, 246 insertions(+), 53 deletions(-) create mode 100644 tests/test_games/AM-subgames.efg create mode 100644 tests/test_games/subgame-roots-finder-multiple-merges.efg create mode 100644 tests/test_games/subgame-roots-finder-one-merge.efg rename tests/test_games/{subgames.efg => subgame-roots-finder-small-subgames-and-merges.efg} (74%) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index cce9a0e50..00b3719f9 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1146,7 +1146,6 @@ void UnionSets(SubgameScratchData &p_data, int p_i, int p_j, GameNodeRep *p_curr // Prefer j's root if it exists, otherwise use current node auto *best_candidate = p_data.leader_subgame_map[leader_j] ? p_data.leader_subgame_map[leader_j] : p_current_node; - p_data.dsu_parent[leader_j] = leader_i; p_data.leader_subgame_map[leader_i] = best_candidate; } @@ -1181,17 +1180,18 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) } } else { - p_data.infoset_visited[curr_id] = true; - } - - // adds other members of the corresponding infoset to the frontier - for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { - auto *member = member_sp.get(); - if (p_data.node_visited_token[member->GetNumber()] != p_data.current_token) { + // Add other members of the corresponding infoset to the frontier + for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { + auto *member = member_sp.get(); + if (member == curr) { + continue; + } local_frontier.push(member); p_data.node_visited_token[member->GetNumber()] = p_data.current_token; } + p_data.infoset_visited[curr_id] = true; } + // if the frontier is not empty and the parent exists, add it to the frontier if (!local_frontier.empty()) { if (auto parent_sp = curr->GetParent()) { @@ -1205,12 +1205,13 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) } } -// Main algorithm: find subgame roots for all infosets +// Find subgame roots for all infosets std::map FindSubgameRoots(const Game &p_game) { if (p_game->GetRoot()->IsTerminal()) { return {}; } + SubgameScratchData data; // Initialize infoset ID mapping using iterators @@ -1260,7 +1261,6 @@ std::map FindSubgameRoots(const Game &p_game) global_frontier.pop(); const int id = data.infoset_to_id[node->GetInfoset().get()]; - if (data.infoset_visited[id]) { continue; } @@ -1268,7 +1268,6 @@ std::map FindSubgameRoots(const Game &p_game) GenerateComponent(data, node); auto *component_top_node = data.leader_subgame_map[FindSet(data, id)]; - if (auto parent_sp = component_top_node->GetParent()) { auto *parent = parent_sp.get(); if (data.node_visited_token[parent->GetNumber()] == 0) { diff --git a/tests/test_games/AM-subgames.efg b/tests/test_games/AM-subgames.efg new file mode 100644 index 000000000..7adfe10ce --- /dev/null +++ b/tests/test_games/AM-subgames.efg @@ -0,0 +1,14 @@ +EFG 2 R "Untitled Extensive Game" { "Player 1" "Player 2" } +"" + +p "" 1 1 "" { "1" "2" } 0 +p "" 1 1 "" { "1" "2" } 0 +p "" 2 1 "" { "1" "2" } 0 +t "" 1 "Outcome 1" { 1, -1 } +t "" 2 "Outcome 2" { 2, -2 } +p "" 2 3 "" { "1" "2" } 0 +t "" 3 "Outcome 3" { 3, -3 } +t "" 4 "Outcome 4" { 4, -4 } +p "" 2 2 "" { "1" "2" } 0 +t "" 5 "Outcome 5" { 5, -5 } +t "" 6 "Outcome 6" { 6, -6 } diff --git a/tests/test_games/subgame-roots-finder-multiple-merges.efg b/tests/test_games/subgame-roots-finder-multiple-merges.efg new file mode 100644 index 000000000..bb6c6b0f0 --- /dev/null +++ b/tests/test_games/subgame-roots-finder-multiple-merges.efg @@ -0,0 +1,55 @@ +EFG 2 R "Untitled Extensive Game" { "Player 1" "Player 2" } +"" + +p "" 1 1 "" { "1" "1" } 0 +t "" 1 "Outcome 1" { 1, -1 } +p "" 2 1 "" { "1" "1" "1" "1" "1" } 0 +p "" 1 2 "" { "1" "1" } 0 +t "" 2 "Outcome 2" { 2, -2 } +p "" 1 3 "" { "1" "1" } 0 +t "" 3 "Outcome 3" { 3, -3 } +t "" 4 "Outcome 4" { 4, -4 } +p "" 1 3 "" { "1" "1" } 0 +p "" 1 2 "" { "1" "1" } 0 +t "" 5 "Outcome 5" { 5, -5 } +t "" 6 "Outcome 6" { 6, -6 } +t "" 7 "Outcome 7" { 7, -7 } +p "" 2 2 "" { "1" "1" } 0 +t "" 8 "Outcome 8" { 8, -8 } +p "" 1 4 "" { "1" "1" } 0 +p "" 2 3 "" { "1" "1" } 0 +t "" 9 "Outcome 9" { 9, -9 } +t "" 10 "Outcome 10" { 10, -10 } +p "" 2 4 "" { "1" "1" } 0 +t "" 11 "Outcome 11" { 11, -11 } +p "" 1 5 "" { "1" "1" } 0 +p "" 1 6 "" { "1" "1" } 0 +p "" 2 5 "" { "1" "1" } 0 +t "" 12 "Outcome 12" { 12, -12 } +t "" 13 "Outcome 13" { 13, -13 } +t "" 14 "Outcome 14" { 14, -14 } +p "" 1 6 "" { "1" "1" } 0 +p "" 2 6 "" { "1" "1" } 0 +c "" 1 "" { "1" 1/2 "1" 1/2 } 0 +p "" 2 5 "" { "1" "1" } 0 +p "" 2 3 "" { "1" "1" } 0 +t "" 15 "Outcome 15" { 15, -15 } +t "" 16 "Outcome 16" { 16, -16 } +t "" 17 "Outcome 17" { 17, -17 } +p "" 2 5 "" { "1" "1" } 0 +t "" 18 "Outcome 18" { 18, -18 } +t "" 19 "Outcome 19" { 19, -19 } +t "" 20 "Outcome 20" { 20, -20 } +p "" 2 6 "" { "1" "1" } 0 +p "" 2 7 "" { "1" "1" } 0 +t "" 21 "Outcome 21" { 21, -21 } +t "" 22 "Outcome 22" { 22, -22 } +p "" 2 7 "" { "1" "1" } 0 +t "" 23 "Outcome 23" { 23, -23 } +t "" 24 "Outcome 24" { 24, -24 } +p "" 1 7 "" { "1" "1" } 0 +t "" 25 "Outcome 25" { 25, -25 } +t "" 26 "Outcome 26" { 26, -26 } +p "" 1 7 "" { "1" "1" } 0 +t "" 27 "Outcome 27" { 27, -27 } +t "" 28 "Outcome 28" { 28, -28 } diff --git a/tests/test_games/subgame-roots-finder-one-merge.efg b/tests/test_games/subgame-roots-finder-one-merge.efg new file mode 100644 index 000000000..83d6fa7ad --- /dev/null +++ b/tests/test_games/subgame-roots-finder-one-merge.efg @@ -0,0 +1,28 @@ +EFG 2 R "Untitled Extensive Game" { "Player 1" "Player 2" } +"" + +p "" 1 1 "" { "1" "1" } 0 +t "" 1 "Outcome 1" { 0, 0 } +p "" 1 2 "" { "1" "1" } 0 +p "" 2 1 "" { "1" "1" } 0 +p "" 1 3 "" { "1" "1" } 0 +t "" 2 "Outcome 2" { 1, 1 } +t "" 3 "Outcome 3" { 2, -2 } +t "" 4 "Outcome 4" { 3, -3 } +p "" 2 1 "" { "1" "1" } 0 +p "" 1 4 "" { "1" "1" } 0 +p "" 2 2 "" { "1" "1" } 0 +p "" 1 3 "" { "1" "1" } 0 +t "" 5 "Outcome 5" { 4, -4 } +t "" 6 "Outcome 6" { 5, -5 } +p "" 1 3 "" { "1" "1" } 0 +t "" 7 "Outcome 7" { 7, -7 } +t "" 8 "Outcome 8" { 8, -8 } +t "" 9 "Outcome 9" { 9, -9 } +p "" 1 4 "" { "1" "1" } 0 +p "" 2 3 "" { "1" "1" } 0 +t "" 10 "Outcome 10" { 10, -10 } +t "" 11 "Outcome 11" { 11, -11 } +p "" 2 3 "" { "1" "1" } 0 +t "" 12 "Outcome 12" { 12, -12 } +t "" 13 "Outcome 13" { 13, -13 } diff --git a/tests/test_games/subgames.efg b/tests/test_games/subgame-roots-finder-small-subgames-and-merges.efg similarity index 74% rename from tests/test_games/subgames.efg rename to tests/test_games/subgame-roots-finder-small-subgames-and-merges.efg index 1e1d21278..e54ada511 100644 --- a/tests/test_games/subgames.efg +++ b/tests/test_games/subgame-roots-finder-small-subgames-and-merges.efg @@ -9,22 +9,30 @@ p "" 1 2 "" { "1" "2" } 0 p "" 2 2 "" { "1" "2" } 0 t "" 3 "Outcome 3" { 3, -3 } p "" 1 3 "" { "1" "2" } 0 +p "" 2 3 "" { "1" "1" } 0 t "" 4 "Outcome 4" { 4, -4 } +p "" 2 4 "" { "1" "1" } 0 +t "" 20 "Outcome 20" { 20, -20 } +t "" 21 "Outcome 21" { 21, -21 } +p "" 2 4 "" { "1" "1" } 0 +p "" 2 3 "" { "1" "1" } 0 t "" 5 "Outcome 5" { 5, -5 } +t "" 22 "Outcome 22" { 22, -22 } +t "" 23 "Outcome 23" { 23, -23 } p "" 2 2 "" { "1" "2" } 0 -p "" 2 3 "" { "1" "2" } 0 +p "" 2 5 "" { "1" "2" } 0 p "" 1 4 "" { "1" "2" } 0 -p "" 2 4 "" { "1" "2" } 0 +p "" 2 6 "" { "1" "2" } 0 t "" 6 "Outcome 6" { 6, -6 } t "" 7 "Outcome 7" { 7, -7 } t "" 8 "Outcome 8" { 8, -8 } p "" 1 5 "" { "1" "2" } 0 -p "" 2 6 "" { "1" "2" } 0 +p "" 2 7 "" { "1" "2" } 0 t "" 9 "Outcome 9" { 9, -9 } t "" 10 "Outcome 10" { 10, -10 } -p "" 2 6 "" { "1" "2" } 0 -p "" 1 7 "" { "1" "2" } 0 -p "" 2 5 "" { "1" "2" } 0 +p "" 2 7 "" { "1" "2" } 0 +p "" 1 6 "" { "1" "2" } 0 +p "" 2 8 "" { "1" "2" } 0 p "" 1 4 "" { "1" "2" } 0 t "" 11 "Outcome 11" { 11, -11 } t "" 12 "Outcome 12" { 12, -12 } @@ -32,9 +40,9 @@ p "" 1 4 "" { "1" "2" } 0 t "" 13 "Outcome 13" { 13, -13 } t "" 14 "Outcome 14" { 14, -14 } t "" 15 "Outcome 15" { 15, -15 } -p "" 1 7 "" { "1" "2" } 0 +p "" 1 6 "" { "1" "2" } 0 t "" 16 "Outcome 16" { 16, -16 } t "" 17 "Outcome 17" { 17, -17 } -p "" 1 6 "" { "1" "2" } 0 +p "" 1 7 "" { "1" "2" } 0 t "" 18 "Outcome 18" { 18, -18 } t "" 19 "Outcome 19" { 19, -19 } diff --git a/tests/test_infosets.py b/tests/test_infosets.py index 4066939d0..86854bfa3 100644 --- a/tests/test_infosets.py +++ b/tests/test_infosets.py @@ -111,21 +111,23 @@ def test_infoset_plays(): ] ), ( - "subgames.efg", + "subgame-roots-finder-small-subgames-and-merges.efg", [ ("Player 1", 0, {None}), ("Player 1", 1, {None}), ("Player 1", 2, {("Player 1", 1, "1")}), - ("Player 1", 3, {("Player 1", 5, "1"), ("Player 1", 1, "2")}), + ("Player 1", 3, {("Player 1", 1, "2"), ("Player 1", 5, "1")}), ("Player 1", 4, {("Player 1", 1, "2")}), ("Player 1", 5, {("Player 1", 4, "2")}), ("Player 1", 6, {("Player 1", 1, "2")}), ("Player 2", 0, {None}), ("Player 2", 1, {("Player 2", 0, "2")}), - ("Player 2", 2, {("Player 2", 1, "1")}), - ("Player 2", 3, {("Player 2", 2, "1")}), - ("Player 2", 4, {("Player 2", 2, "2")}), + ("Player 2", 2, {("Player 2", 1, "2"), ("Player 2", 3, "1")}), + ("Player 2", 3, {("Player 2", 2, "1"), ("Player 2", 1, "2")}), + ("Player 2", 4, {("Player 2", 1, "1")}), ("Player 2", 5, {("Player 2", 4, "1")}), + ("Player 2", 6, {("Player 2", 4, "2")}), + ("Player 2", 7, {("Player 2", 6, "1")}), ] ), # An absent-minded driver game diff --git a/tests/test_node.py b/tests/test_node.py index 62309134a..f163ebdab 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -137,22 +137,26 @@ def _get_path_of_action_labels(node: gbt.Node) -> list[str]: ] ), ( - "subgames.efg", + "subgame-roots-finder-small-subgames-and-merges.efg", [ ([], None), (["1"], None), (["2"], None), (["1", "2"], ("Player 2", 0, "2")), (["2", "1", "2"], ("Player 1", 1, "1")), + (["1", "2", "1", "2"], ("Player 2", 1, "2")), + (["1", "1", "2", "1", "2"], ("Player 2", 2, "1")), + (["2", "2", "1", "2"], ("Player 2", 1, "2")), + (["1", "2", "2", "1", "2"], ("Player 2", 3, "1")), (["2", "2"], ("Player 2", 0, "2")), (["1", "2", "2"], ("Player 2", 1, "1")), (["1", "1", "2", "2"], ("Player 1", 1, "2")), - (["1", "1", "1", "2", "2"], ("Player 2", 2, "1")), + (["1", "1", "1", "2", "2"], ("Player 2", 4, "1")), (["2", "1", "2", "2"], ("Player 1", 1, "2")), - (["1", "2", "1", "2", "2"], ("Player 2", 2, "2")), - (["2", "2", "1", "2", "2"], ("Player 2", 2, "2")), + (["1", "2", "1", "2", "2"], ("Player 2", 4, "2")), + (["2", "2", "1", "2", "2"], ("Player 2", 4, "2")), (["1", "2", "2", "1", "2", "2"], ("Player 1", 4, "2")), - (["1", "1", "2", "2", "1", "2", "2"], ("Player 2", 4, "1")), + (["1", "1", "2", "2", "1", "2", "2"], ("Player 2", 6, "1")), (["1", "1", "1", "2", "2", "1", "2", "2"], ("Player 1", 5, "1")), (["2", "1", "1", "2", "2", "1", "2", "2"], ("Player 1", 5, "1")), (["2", "2", "2", "1", "2", "2"], ("Player 1", 4, "2")), @@ -202,11 +206,15 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): # Games without Absent-Mindedness for which the legacy method is known to be correct. (games.read_from_file("wichardt.efg"), {0}), (games.read_from_file("e02.efg"), {0, 2, 4}), - (games.read_from_file("merge.efg"), {0, 2}), - (games.read_from_file("sg.efg"), {0, 1, 4, 7, 11, 13, 34}), + (games.read_from_file("subgame-roots-finder-one-merge.efg"), {0, 2}), + ( + games.read_from_file("subgame-roots-finder-multiple-roots-and-merge.efg"), + {0, 1, 4, 7, 11, 13, 34} + ), (games.read_from_file("subgame-8-roots.efg"), {0, 1, 2, 4, 5, 16, 27, 30}), (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], {0, 2, 4, 6, 8}), (gbt.Game.new_tree(), {}), + (games.read_from_file("subgame-roots-finder-multiple-merges.efg"), {0, 2, 13, 15}), # Games with Absent-Mindedness where the legacy method is known to fail. pytest.param( @@ -229,49 +237,128 @@ def test_legacy_is_subgame_root_set(game: gbt.Game, expected_result: set): # NEW subgame finder algorithm -@pytest.mark.parametrize("game, expected_result", [ - # Games without Absent-Mindedness. - (games.read_from_file("e02.efg"), {0, 2, 4}), - (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], {0, 2, 4, 6, 8}), - (games.read_from_file("wichardt.efg"), {0}), - (games.read_from_file("merge.efg"), {0, 2}), - (games.read_from_file("sg.efg"), {0, 1, 4, 7, 11, 13, 34}), - (games.read_from_file("subgame-8-roots.efg"), {0, 1, 2, 4, 5, 16, 27, 30}), - (gbt.Game.new_tree(), {}), +@pytest.mark.parametrize("game, expected_paths_list", [ + # Empty game has no subgames + ( + gbt.Game.new_tree(), + [] + ), + # --- Games without Absent-Mindedness. --- + # Perfect Information + ( + games.read_from_file("e02.efg"), + [ + [], + ["L"], + ["L", "L"] + ] + ), + ( + games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], + [ + ["Push", "Push"], + ["Push", "Push", "Push", "Push"], + ["Push", "Push", "Push"], + ["Push"], + [] + ] + ), + # Perfect Recall + ( + games.read_from_file("binary_3_levels_generic_payoffs.efg"), + [ + [] + ] + ), + # No perfect recall + ( + games.read_from_file("wichardt.efg"), + [ + [] + ] + ), + ( + games.read_from_file("subgame-roots-finder-one-merge.efg"), + [ + [], + ["1"] + ] + ), + ( + games.read_from_file("subgame-roots-finder-small-subgames-and-merges.efg"), + [ + ["2"], + ["1"], + ["1", "2", "2"], + ["2", "1", "2"], + [], + ["1", "1", "1", "2", "2"], + ["2", "2", "2"] + ] + ), + ( + games.read_from_file("subgame-roots-finder-multiple-merges.efg"), + [ + [], + ["1", "1"], + ["1"], + ["1", "1", "1"] + ] + ), - # Games with Absent-Mindedness, which the new method handles correctly. - (games.read_from_file("AM-subgames.efg"), {0, 2, 5, 8}), + # --- Games with Absent-Mindedness. --- + ( + games.read_from_file("AM-subgames.efg"), + [ + [], + ["1", "1"], + ["2"], + ["2", "1"] + ] + ), + ( + games.read_from_file("noPR-action-AM-two-hops.efg"), + [ + [], + ["2", "1", "1"] + ] + ), ]) -def test_new_subgame_root_method_set(game: gbt.Game, expected_result: set): +def test_new_subgame_root_method_paths(game: gbt.Game, expected_paths_list: list): """ - Tests the new `game.subgame_root()` method against an expected set of nodes. - This method is expected to pass all cases, including those with Absent-Mindedness. + Tests `game.subgame_root()` by comparing the paths of the identified root nodes + against the expected paths. """ - list_nodes = list(game.nodes) - expected_roots = {list_nodes[i] for i in expected_result} + actual_root_nodes = { + game.subgame_root(infoset) + for infoset in game.infosets + if not infoset.is_chance + } - actual_roots = {game.subgame_root(infoset) - for infoset in game.infosets if not infoset.is_chance} + actual_paths = [ + _get_path_of_action_labels(node) + for node in actual_root_nodes + ] - assert actual_roots == expected_roots + assert sorted(actual_paths) == sorted(expected_paths_list) @pytest.mark.parametrize("game_file, expected_unreachable_paths", [ # Games without absent-mindedness, where all nodes are reachable ("e02.efg", []), ("wichardt.efg", []), - ("subgames.efg", []), + ("subgame-roots-finder-small-subgames-and-merges.efg", []), # An absent-minded driver game with an unreachable terminal node ( "AM-driver-one-infoset.efg", - [["S", "T"]] + [["T", "S"]] ), # An absent-minded driver game with an unreachable subtree ( "AM-driver-subgame.efg", - [["S", "T"], ["S", "T", "r"], ["S", "T", "l"]] + [["T", "S"], ["r", "T", "S"], ["l", "T", "S"]] ), ]) def test_is_strategy_reachable(game_file: str, expected_unreachable_paths: list[list[str]]): From 9bd1e26cb8bd805167d055d62e2701ab5ee03db6 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 4 Dec 2025 11:22:54 +0000 Subject: [PATCH 03/18] Add missing .efg file --- ...-roots-finder-multiple-roots-and-merge.efg | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 tests/test_games/subgame-roots-finder-multiple-roots-and-merge.efg diff --git a/tests/test_games/subgame-roots-finder-multiple-roots-and-merge.efg b/tests/test_games/subgame-roots-finder-multiple-roots-and-merge.efg new file mode 100644 index 000000000..1e1d21278 --- /dev/null +++ b/tests/test_games/subgame-roots-finder-multiple-roots-and-merge.efg @@ -0,0 +1,40 @@ +EFG 2 R "Untitled Extensive Game" { "Player 1" "Player 2" } +"" + +p "" 2 1 "" { "1" "2" } 0 +p "" 1 1 "" { "1" "2" } 0 +t "" 1 "Outcome 1" { 1, -1 } +t "" 2 "Outcome 2" { 2, -2 } +p "" 1 2 "" { "1" "2" } 0 +p "" 2 2 "" { "1" "2" } 0 +t "" 3 "Outcome 3" { 3, -3 } +p "" 1 3 "" { "1" "2" } 0 +t "" 4 "Outcome 4" { 4, -4 } +t "" 5 "Outcome 5" { 5, -5 } +p "" 2 2 "" { "1" "2" } 0 +p "" 2 3 "" { "1" "2" } 0 +p "" 1 4 "" { "1" "2" } 0 +p "" 2 4 "" { "1" "2" } 0 +t "" 6 "Outcome 6" { 6, -6 } +t "" 7 "Outcome 7" { 7, -7 } +t "" 8 "Outcome 8" { 8, -8 } +p "" 1 5 "" { "1" "2" } 0 +p "" 2 6 "" { "1" "2" } 0 +t "" 9 "Outcome 9" { 9, -9 } +t "" 10 "Outcome 10" { 10, -10 } +p "" 2 6 "" { "1" "2" } 0 +p "" 1 7 "" { "1" "2" } 0 +p "" 2 5 "" { "1" "2" } 0 +p "" 1 4 "" { "1" "2" } 0 +t "" 11 "Outcome 11" { 11, -11 } +t "" 12 "Outcome 12" { 12, -12 } +p "" 1 4 "" { "1" "2" } 0 +t "" 13 "Outcome 13" { 13, -13 } +t "" 14 "Outcome 14" { 14, -14 } +t "" 15 "Outcome 15" { 15, -15 } +p "" 1 7 "" { "1" "2" } 0 +t "" 16 "Outcome 16" { 16, -16 } +t "" 17 "Outcome 17" { 17, -17 } +p "" 1 6 "" { "1" "2" } 0 +t "" 18 "Outcome 18" { 18, -18 } +t "" 19 "Outcome 19" { 19, -19 } From d3e71728347b0ef3502ca3e4d30b6db5a2fe9fc9 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 4 Dec 2025 11:43:24 +0000 Subject: [PATCH 04/18] Delete an outdated subgame roots test in the legacy test suite --- tests/test_node.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_node.py b/tests/test_node.py index f163ebdab..bc78577a7 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -211,7 +211,6 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): games.read_from_file("subgame-roots-finder-multiple-roots-and-merge.efg"), {0, 1, 4, 7, 11, 13, 34} ), - (games.read_from_file("subgame-8-roots.efg"), {0, 1, 2, 4, 5, 16, 27, 30}), (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], {0, 2, 4, 6, 8}), (gbt.Game.new_tree(), {}), (games.read_from_file("subgame-roots-finder-multiple-merges.efg"), {0, 2, 13, 15}), From e58f7e9cd71d1d0c964f98847d566685ab84f9bb Mon Sep 17 00:00:00 2001 From: drdkad Date: Sat, 6 Dec 2025 08:56:27 +0000 Subject: [PATCH 05/18] Refactor Subgame Root Finder: replace IDs with object pointers and improve DSU --- src/games/game.h | 3 +- src/games/gameagg.h | 2 +- src/games/gamebagg.h | 2 +- src/games/gametable.h | 2 +- src/games/gametree.cc | 161 +++++++++++++++++++----------------------- src/games/gametree.h | 4 +- 6 files changed, 80 insertions(+), 94 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index 57f92b17f..99d860edc 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -432,7 +432,6 @@ class GameNodeRep : public std::enable_shared_from_this { friend class GameInfosetRep; friend class GamePlayerRep; friend class PureBehaviorProfile; - friend class SubgameRootFinder; template friend class MixedBehaviorProfile; bool m_valid{true}; @@ -838,7 +837,7 @@ class GameRep : public std::enable_shared_from_this { return false; } /// - virtual GameNode GetSubgameRoot(GameInfoset) const = 0; + virtual GameNode GetSubgameRoot(const GameInfoset &) const = 0; //@} /// @name Writing data files diff --git a/src/games/gameagg.h b/src/games/gameagg.h index e48f782e0..f16d6c97e 100644 --- a/src/games/gameagg.h +++ b/src/games/gameagg.h @@ -81,7 +81,7 @@ class GameAGGRep : public GameRep { bool IsTree() const override { return false; } bool IsAgg() const override { return true; } bool IsPerfectRecall() const override { return true; } - GameNode GetSubgameRoot(GameInfoset) const override { return {nullptr}; } + GameNode GetSubgameRoot(const GameInfoset &) const override { return {nullptr}; } bool IsConstSum() const override; /// Returns the smallest payoff to any player in any outcome of the game Rational GetMinPayoff() const override { return Rational(aggPtr->getMinPayoff()); } diff --git a/src/games/gamebagg.h b/src/games/gamebagg.h index f976dca38..9d6cc4680 100644 --- a/src/games/gamebagg.h +++ b/src/games/gamebagg.h @@ -88,7 +88,7 @@ class GameBAGGRep : public GameRep { bool IsTree() const override { return false; } virtual bool IsBagg() const { return true; } bool IsPerfectRecall() const override { return true; } - GameNode GetSubgameRoot(GameInfoset) const override { return {nullptr}; } + GameNode GetSubgameRoot(const GameInfoset &) const override { return {nullptr}; } bool IsConstSum() const override { throw UndefinedException(); } /// Returns the smallest payoff to any player in any outcome of the game Rational GetMinPayoff() const override { return Rational(baggPtr->getMinPayoff()); } diff --git a/src/games/gametable.h b/src/games/gametable.h index edc25687e..445b4c443 100644 --- a/src/games/gametable.h +++ b/src/games/gametable.h @@ -64,7 +64,7 @@ class GameTableRep : public GameExplicitRep { Rational GetPlayerMaxPayoff(const GamePlayer &) const override; bool IsPerfectRecall() const override { return true; } - GameNode GetSubgameRoot(GameInfoset) const override { return {nullptr}; } + GameNode GetSubgameRoot(const GameInfoset &) const override { return {nullptr}; } //@} /// @name Dimensions of the game diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 00b3719f9..ceed83cc9 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -885,6 +885,7 @@ void GameTreeRep::ClearComputedValues() const m_ownPriorActionInfo = nullptr; const_cast(this)->m_unreachableNodes = nullptr; m_absentMindedInfosets.clear(); + m_infosetSubgameRoot.clear(); m_computedValues = false; } @@ -1097,8 +1098,7 @@ void GameTreeRep::BuildUnreachableNodes() const //------------------------------------------------------------------------ // Subgame Root Finder //------------------------------------------------------------------------ - -namespace { // Anonymous namespace for internal implementation +namespace { // Anonymous namespace // Comparator for priority queue (ancestors have lower priority than descendants) struct NodeCmp { @@ -1110,73 +1110,86 @@ struct NodeCmp { // Scratch data structure for subgame root finding algorithm struct SubgameScratchData { - // DSU structures - std::vector dsu_parent; - std::vector leader_subgame_map; + std::map dsu_parent; + std::map subgame_root_candidate; + std::map visited; + std::map node_visited_token; + int current_token = 0; - // ID mapping - std::map infoset_to_id; + // Iterative DSU Find + path compression + GameInfosetRep *FindSet(GameInfosetRep *p_infoset) + { + // Initialize if not present + if (dsu_parent.find(p_infoset) == dsu_parent.end()) { + dsu_parent[p_infoset] = p_infoset; + } - // Visited tracking - std::vector infoset_visited; - std::vector node_visited_token; - int current_token = 0; -}; + // Find root + auto *root = p_infoset; + while (dsu_parent[root] != root) { + root = dsu_parent[root]; + } -// DSU Find with path compression -int FindSet(SubgameScratchData &p_data, int p_i) -{ - if (p_data.dsu_parent[p_i] != p_i) { - p_data.dsu_parent[p_i] = FindSet(p_data, p_data.dsu_parent[p_i]); + // Path compression + GameInfosetRep *curr = p_infoset; + while (curr != root) { + auto *next = dsu_parent[curr]; + dsu_parent[curr] = root; + curr = next; + } + + return root; } - return p_data.dsu_parent[p_i]; -} -// DSU Union - merge set j into set i, tracking best subgame root -void UnionSets(SubgameScratchData &p_data, int p_i, int p_j, GameNodeRep *p_current_node) -{ - const int leader_i = FindSet(p_data, p_i); - const int leader_j = FindSet(p_data, p_j); + // DSU Union - merge current infoset into the staring one, tracking best subgame root candidate + void UnionSets(GameInfosetRep *p_start_infoset, GameInfosetRep *p_current_infoset, + GameNodeRep *p_current_node) + { + auto *leader_start = FindSet(p_start_infoset); + auto *leader_current = FindSet(p_current_infoset); - if (leader_i == leader_j) { - p_data.leader_subgame_map[leader_i] = p_current_node; - return; - } + if (leader_start == leader_current) { + subgame_root_candidate[leader_start] = p_current_node; + return; + } - // Prefer j's root if it exists, otherwise use current node - auto *best_candidate = - p_data.leader_subgame_map[leader_j] ? p_data.leader_subgame_map[leader_j] : p_current_node; - p_data.dsu_parent[leader_j] = leader_i; - p_data.leader_subgame_map[leader_i] = best_candidate; -} + // Prefer the current infoset's root if it exists; otherwise use the current node being visited + auto *best_candidate = subgame_root_candidate[leader_current] + ? subgame_root_candidate[leader_current] + : p_current_node; + + dsu_parent[leader_current] = leader_start; + subgame_root_candidate[leader_start] = best_candidate; + } +}; // Generate a single component starting from a given node void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) { - const int start_id = p_data.infoset_to_id[p_start_node->GetInfoset().get()]; + auto *start_infoset = p_start_node->GetInfoset().get(); p_data.current_token++; std::priority_queue, NodeCmp> local_frontier; local_frontier.push(p_start_node); - p_data.node_visited_token[p_start_node->GetNumber()] = p_data.current_token; + p_data.node_visited_token[p_start_node] = p_data.current_token; while (!local_frontier.empty()) { auto *curr = local_frontier.top(); local_frontier.pop(); - const int curr_id = p_data.infoset_to_id[curr->GetInfoset().get()]; + GameInfosetRep *curr_infoset = curr->GetInfoset().get(); - UnionSets(p_data, start_id, curr_id, curr); + p_data.UnionSets(start_infoset, curr_infoset, curr); // External collision: current node belongs to a previously generated component - if (p_data.infoset_visited[curr_id]) { - const int leader = FindSet(p_data, curr_id); - auto *candidate_root = p_data.leader_subgame_map[leader]; + if (p_data.visited[curr_infoset]) { + auto *leader = p_data.FindSet(curr_infoset); + auto *candidate_root = p_data.subgame_root_candidate[leader]; - if (p_data.node_visited_token[candidate_root->GetNumber()] != p_data.current_token) { + if (p_data.node_visited_token[candidate_root] != p_data.current_token) { local_frontier.push(candidate_root); - p_data.node_visited_token[candidate_root->GetNumber()] = p_data.current_token; + p_data.node_visited_token[candidate_root] = p_data.current_token; } } else { @@ -1187,18 +1200,17 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) continue; } local_frontier.push(member); - p_data.node_visited_token[member->GetNumber()] = p_data.current_token; + p_data.node_visited_token[member] = p_data.current_token; } - p_data.infoset_visited[curr_id] = true; + p_data.visited[curr_infoset] = true; } - // if the frontier is not empty and the parent exists, add it to the frontier if (!local_frontier.empty()) { if (auto parent_sp = curr->GetParent()) { auto *parent = parent_sp.get(); - if (p_data.node_visited_token[parent->GetNumber()] != p_data.current_token) { + if (p_data.node_visited_token[parent] != p_data.current_token) { local_frontier.push(parent); - p_data.node_visited_token[parent->GetNumber()] = p_data.current_token; + p_data.node_visited_token[parent] = p_data.current_token; } } } @@ -1213,44 +1225,17 @@ std::map FindSubgameRoots(const Game &p_game) } SubgameScratchData data; + data.current_token++; - // Initialize infoset ID mapping using iterators - int next_id = 0; - - // Map Chance player's infosets - for (const auto &infoset : p_game->GetChance()->GetInfosets()) { - data.infoset_to_id[infoset.get()] = next_id++; - } - - // Map individual player's infosets - for (const auto &player : p_game->GetPlayers()) { - for (const auto &infoset : player->GetInfosets()) { - data.infoset_to_id[infoset.get()] = next_id++; - } - } - - // Initialize DSU structures - const int total_infosets = next_id; - data.dsu_parent.resize(total_infosets); - std::iota(data.dsu_parent.begin(), data.dsu_parent.end(), 0); - data.leader_subgame_map.assign(total_infosets, nullptr); - - // Initialize tracking - data.infoset_visited.assign(total_infosets, false); - data.node_visited_token.assign(p_game->NumNodes() + 1, 0); - - // Build global frontier (priority queue of nodes to process) std::priority_queue, NodeCmp> global_frontier; - data.current_token++; - // Add parents of terminal nodes to the global frontier for (const auto &node : p_game->GetNodes()) { if (node->IsTerminal()) { auto *parent = node->GetParent().get(); - if (data.node_visited_token[parent->GetNumber()] != data.current_token) { + if (data.node_visited_token[parent] != data.current_token) { global_frontier.push(parent); - data.node_visited_token[parent->GetNumber()] = data.current_token; + data.node_visited_token[parent] = data.current_token; } } } @@ -1260,19 +1245,21 @@ std::map FindSubgameRoots(const Game &p_game) auto *node = global_frontier.top(); global_frontier.pop(); - const int id = data.infoset_to_id[node->GetInfoset().get()]; - if (data.infoset_visited[id]) { + auto *infoset = node->GetInfoset().get(); + + if (data.visited[infoset]) { continue; } GenerateComponent(data, node); - auto *component_top_node = data.leader_subgame_map[FindSet(data, id)]; + auto *component_top_node = data.subgame_root_candidate[data.FindSet(infoset)]; + if (auto parent_sp = component_top_node->GetParent()) { auto *parent = parent_sp.get(); - if (data.node_visited_token[parent->GetNumber()] == 0) { + if (data.node_visited_token[parent] == 0) { global_frontier.push(parent); - data.node_visited_token[parent->GetNumber()] = data.current_token; + data.node_visited_token[parent] = data.current_token; } } } @@ -1281,8 +1268,8 @@ std::map FindSubgameRoots(const Game &p_game) auto collect_results = [&](const GamePlayer &p_player) { for (const auto &infoset : p_player->GetInfosets()) { - const int id = data.infoset_to_id[infoset.get()]; - result[infoset.get()] = data.leader_subgame_map[FindSet(data, id)]; + auto *ptr = infoset.get(); + result[ptr] = data.subgame_root_candidate[data.FindSet(ptr)]; } }; @@ -1296,12 +1283,12 @@ std::map FindSubgameRoots(const Game &p_game) } // end anonymous namespace -void GameTreeRep::BuildSubgameRoots() +void GameTreeRep::BuildSubgameRoots() const { if (!m_computedValues) { BuildComputedValues(); } - m_infosetSubgameRoot = FindSubgameRoots(shared_from_this()); + m_infosetSubgameRoot = FindSubgameRoots(std::const_pointer_cast(shared_from_this())); } //------------------------------------------------------------------------ diff --git a/src/games/gametree.h b/src/games/gametree.h index 3754aa9bc..2b2d34f65 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -48,7 +48,7 @@ class GameTreeRep : public GameExplicitRep { mutable std::shared_ptr m_ownPriorActionInfo; mutable std::unique_ptr> m_unreachableNodes; mutable std::set m_absentMindedInfosets; - std::map m_infosetSubgameRoot; + mutable std::map m_infosetSubgameRoot; /// @name Private auxiliary functions //@{ @@ -182,7 +182,7 @@ class GameTreeRep : public GameExplicitRep { std::vector BuildConsistentPlaysRecursiveImpl(GameNodeRep *node); void BuildOwnPriorActions() const; void BuildUnreachableNodes() const; - void BuildSubgameRoots(); + void BuildSubgameRoots() const; }; template class TreeMixedStrategyProfileRep : public MixedStrategyProfileRep { From 49bb98e69b3cba56ce6474c9029c5765efd8b059 Mon Sep 17 00:00:00 2001 From: drdkad Date: Sat, 6 Dec 2025 19:01:15 +0000 Subject: [PATCH 06/18] Refactor the legacy implementation of IsSubgameRoot --- src/games/gametree.cc | 29 +++-------------------------- 1 file changed, 3 insertions(+), 26 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index ceed83cc9..8dfb957a7 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -372,32 +372,10 @@ bool GameNodeRep::IsSuccessorOf(GameNode p_node) const bool GameNodeRep::IsSubgameRoot() const { - // First take care of a couple easy cases - if (m_children.empty() || m_infoset->m_members.size() > 1) { + if (m_children.empty()) { return false; } - if (!m_parent) { - return true; - } - - // A node is a subgame root if and only if in every information set, - // either all members succeed the node in the tree, - // or all members do not succeed the node in the tree. - for (auto player : m_game->GetPlayers()) { - for (auto infoset : player->GetInfosets()) { - const bool precedes = infoset->m_members.front()->IsSuccessorOf( - std::const_pointer_cast(shared_from_this())); - if (std::any_of(std::next(infoset->m_members.begin()), infoset->m_members.end(), - [this, precedes](const std::shared_ptr &m) { - return m->IsSuccessorOf(std::const_pointer_cast( - shared_from_this())) != precedes; - })) { - return false; - } - } - } - - return true; + return m_game->GetSubgameRoot(m_infoset->shared_from_this()) == shared_from_this(); } bool GameNodeRep::IsStrategyReachable() const @@ -1184,8 +1162,7 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) // External collision: current node belongs to a previously generated component if (p_data.visited[curr_infoset]) { - auto *leader = p_data.FindSet(curr_infoset); - auto *candidate_root = p_data.subgame_root_candidate[leader]; + auto *candidate_root = p_data.subgame_root_candidate[p_data.FindSet(curr_infoset)]; if (p_data.node_visited_token[candidate_root] != p_data.current_token) { local_frontier.push(candidate_root); From 861beaea934f02da1181f6f8a9799db56e627d02 Mon Sep 17 00:00:00 2001 From: drdkad Date: Sun, 7 Dec 2025 08:13:26 +0000 Subject: [PATCH 07/18] Eliminated visited tracking for infosets --- src/games/gametree.cc | 56 +++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 8dfb957a7..c551265e1 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1088,38 +1088,34 @@ struct NodeCmp { // Scratch data structure for subgame root finding algorithm struct SubgameScratchData { + // DSU structures std::map dsu_parent; std::map subgame_root_candidate; - std::map visited; + // Timestamps for nodes std::map node_visited_token; int current_token = 0; - // Iterative DSU Find + path compression + // DSU Find with path compression GameInfosetRep *FindSet(GameInfosetRep *p_infoset) { - // Initialize if not present - if (dsu_parent.find(p_infoset) == dsu_parent.end()) { - dsu_parent[p_infoset] = p_infoset; - } - - // Find root - auto *root = p_infoset; - while (dsu_parent[root] != root) { - root = dsu_parent[root]; + // Initialize and retrieve current parent in one step. + auto *leader = dsu_parent.try_emplace(p_infoset, p_infoset).first->second; + while (dsu_parent[leader] != leader) { + leader = dsu_parent[leader]; } // Path compression - GameInfosetRep *curr = p_infoset; - while (curr != root) { - auto *next = dsu_parent[curr]; - dsu_parent[curr] = root; - curr = next; + auto *curr = p_infoset; + while (curr != leader) { + auto &parent_ref = dsu_parent[curr]; + curr = parent_ref; + parent_ref = leader; } - return root; + return leader; } - // DSU Union - merge current infoset into the staring one, tracking best subgame root candidate + // DSU Union - merge current infoset into the start one, find best subgame candidate root void UnionSets(GameInfosetRep *p_start_infoset, GameInfosetRep *p_current_infoset, GameNodeRep *p_current_node) { @@ -1132,9 +1128,8 @@ struct SubgameScratchData { } // Prefer the current infoset's root if it exists; otherwise use the current node being visited - auto *best_candidate = subgame_root_candidate[leader_current] - ? subgame_root_candidate[leader_current] - : p_current_node; + auto *existing_candidate = subgame_root_candidate[leader_current]; + auto *best_candidate = existing_candidate ? existing_candidate : p_current_node; dsu_parent[leader_current] = leader_start; subgame_root_candidate[leader_start] = best_candidate; @@ -1155,22 +1150,21 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) while (!local_frontier.empty()) { auto *curr = local_frontier.top(); local_frontier.pop(); - - GameInfosetRep *curr_infoset = curr->GetInfoset().get(); + auto *curr_infoset = curr->GetInfoset().get(); + const bool is_external_collision = p_data.dsu_parent.count(curr_infoset); p_data.UnionSets(start_infoset, curr_infoset, curr); - // External collision: current node belongs to a previously generated component - if (p_data.visited[curr_infoset]) { + if (is_external_collision) { + // We hit a node belonging to a previously generated component. auto *candidate_root = p_data.subgame_root_candidate[p_data.FindSet(curr_infoset)]; - if (p_data.node_visited_token[candidate_root] != p_data.current_token) { local_frontier.push(candidate_root); p_data.node_visited_token[candidate_root] = p_data.current_token; } } else { - // Add other members of the corresponding infoset to the frontier + // First time seeing this infoset: add all its members to the frontier. for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { auto *member = member_sp.get(); if (member == curr) { @@ -1179,7 +1173,6 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) local_frontier.push(member); p_data.node_visited_token[member] = p_data.current_token; } - p_data.visited[curr_infoset] = true; } if (!local_frontier.empty()) { @@ -1194,7 +1187,7 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) } } -// Find subgame roots for all infosets +// For each infoset, find the root of the smallest subgame containing it std::map FindSubgameRoots(const Game &p_game) { if (p_game->GetRoot()->IsTerminal()) { @@ -1217,21 +1210,20 @@ std::map FindSubgameRoots(const Game &p_game) } } - // Process components in descending node order while (!global_frontier.empty()) { auto *node = global_frontier.top(); global_frontier.pop(); auto *infoset = node->GetInfoset().get(); - if (data.visited[infoset]) { + // If infoset is already in dsu_parent, it has been processed. + if (data.dsu_parent.count(infoset)) { continue; } GenerateComponent(data, node); auto *component_top_node = data.subgame_root_candidate[data.FindSet(infoset)]; - if (auto parent_sp = component_top_node->GetParent()) { auto *parent = parent_sp.get(); if (data.node_visited_token[parent] == 0) { From e9ca49b4c8b96eb5c58742563f3dbf5cc32dc12f Mon Sep 17 00:00:00 2001 From: drdkad Date: Mon, 8 Dec 2025 07:52:04 +0000 Subject: [PATCH 08/18] Refactor: Replace priority queues with indexed arrays --- src/games/gametree.cc | 117 +++++++++++++++++++----------------------- 1 file changed, 53 insertions(+), 64 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index c551265e1..784a08edc 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1076,34 +1076,28 @@ void GameTreeRep::BuildUnreachableNodes() const //------------------------------------------------------------------------ // Subgame Root Finder //------------------------------------------------------------------------ -namespace { // Anonymous namespace -// Comparator for priority queue (ancestors have lower priority than descendants) -struct NodeCmp { - bool operator()(const GameNodeRep *a, const GameNodeRep *b) const - { - return a->GetNumber() < b->GetNumber(); - } -}; +namespace { // Anonymous implementation namespace // Scratch data structure for subgame root finding algorithm struct SubgameScratchData { // DSU structures - std::map dsu_parent; - std::map subgame_root_candidate; + std::unordered_map dsu_parent; + std::unordered_map subgame_root_candidate; // Timestamps for nodes - std::map node_visited_token; + std::vector node_visited_token; int current_token = 0; + std::vector local_frontier; + // DSU Find with path compression GameInfosetRep *FindSet(GameInfosetRep *p_infoset) { - // Initialize and retrieve current parent in one step. + // Initialize/retrieve current parent auto *leader = dsu_parent.try_emplace(p_infoset, p_infoset).first->second; while (dsu_parent[leader] != leader) { leader = dsu_parent[leader]; } - // Path compression auto *curr = p_infoset; while (curr != leader) { @@ -1111,11 +1105,10 @@ struct SubgameScratchData { curr = parent_ref; parent_ref = leader; } - return leader; } - // DSU Union - merge current infoset into the start one, find best subgame candidate root + // DSU Union - merge current infoset into the start one and update the root candidate void UnionSets(GameInfosetRep *p_start_infoset, GameInfosetRep *p_current_infoset, GameNodeRep *p_current_node) { @@ -1127,10 +1120,8 @@ struct SubgameScratchData { return; } - // Prefer the current infoset's root if it exists; otherwise use the current node being visited auto *existing_candidate = subgame_root_candidate[leader_current]; auto *best_candidate = existing_candidate ? existing_candidate : p_current_node; - dsu_parent[leader_current] = leader_start; subgame_root_candidate[leader_start] = best_candidate; } @@ -1142,48 +1133,61 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) auto *start_infoset = p_start_node->GetInfoset().get(); p_data.current_token++; - std::priority_queue, NodeCmp> local_frontier; + auto &frontier = p_data.local_frontier; + size_t active_nodes_counter = 0; + + const int max_node_number = p_start_node->GetNumber(); + int min_node_number = p_start_node->GetNumber(); // is dynamically updated as the frontier grows + + // Add node to frontier and track minimum + auto push_node = [&](GameNodeRep *node) { + const int node_num = node->GetNumber(); + min_node_number = std::min(min_node_number, node_num); + frontier[node_num] = node; + p_data.node_visited_token[node_num] = p_data.current_token; + active_nodes_counter++; + }; + + push_node(p_start_node); - local_frontier.push(p_start_node); - p_data.node_visited_token[p_start_node] = p_data.current_token; + // Process nodes in descending order + for (int node_num = max_node_number; node_num >= min_node_number; --node_num) { + auto *curr = frontier[node_num]; + if (!curr) { + continue; // Skip empty slots + } + + active_nodes_counter--; - while (!local_frontier.empty()) { - auto *curr = local_frontier.top(); - local_frontier.pop(); auto *curr_infoset = curr->GetInfoset().get(); const bool is_external_collision = p_data.dsu_parent.count(curr_infoset); p_data.UnionSets(start_infoset, curr_infoset, curr); if (is_external_collision) { - // We hit a node belonging to a previously generated component. auto *candidate_root = p_data.subgame_root_candidate[p_data.FindSet(curr_infoset)]; - if (p_data.node_visited_token[candidate_root] != p_data.current_token) { - local_frontier.push(candidate_root); - p_data.node_visited_token[candidate_root] = p_data.current_token; + if (p_data.node_visited_token[candidate_root->GetNumber()] != p_data.current_token) { + push_node(candidate_root); } } else { - // First time seeing this infoset: add all its members to the frontier. for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { auto *member = member_sp.get(); if (member == curr) { continue; } - local_frontier.push(member); - p_data.node_visited_token[member] = p_data.current_token; + push_node(member); } } - if (!local_frontier.empty()) { - if (auto parent_sp = curr->GetParent()) { - auto *parent = parent_sp.get(); - if (p_data.node_visited_token[parent] != p_data.current_token) { - local_frontier.push(parent); - p_data.node_visited_token[parent] = p_data.current_token; - } + if (active_nodes_counter > 0) { + auto *parent = curr->GetParent().get(); + if (p_data.node_visited_token[parent->GetNumber()] != p_data.current_token) { + push_node(parent); } } + + frontier[node_num] = nullptr; } } @@ -1196,41 +1200,26 @@ std::map FindSubgameRoots(const Game &p_game) SubgameScratchData data; data.current_token++; + const int num_nodes = p_game->NumNodes(); - std::priority_queue, NodeCmp> global_frontier; + // Allocate local frontier once for reuse across all component generations + data.local_frontier.resize(num_nodes + 1, nullptr); + data.node_visited_token.resize(num_nodes + 1, 0); - // Add parents of terminal nodes to the global frontier + // Collect nodes in ascending order (depth-first traversal) + std::vector nodes_ascending; + nodes_ascending.reserve(num_nodes); for (const auto &node : p_game->GetNodes()) { - if (node->IsTerminal()) { - auto *parent = node->GetParent().get(); - if (data.node_visited_token[parent] != data.current_token) { - global_frontier.push(parent); - data.node_visited_token[parent] = data.current_token; - } - } + nodes_ascending.push_back(node.get()); } - while (!global_frontier.empty()) { - auto *node = global_frontier.top(); - global_frontier.pop(); - - auto *infoset = node->GetInfoset().get(); - - // If infoset is already in dsu_parent, it has been processed. - if (data.dsu_parent.count(infoset)) { + // Process in reverse order (descending node numbers, children before parents) + for (auto it = nodes_ascending.rbegin(); it != nodes_ascending.rend(); ++it) { + auto *node = *it; + if (node->IsTerminal() || data.dsu_parent.count(node->GetInfoset().get())) { continue; } - GenerateComponent(data, node); - - auto *component_top_node = data.subgame_root_candidate[data.FindSet(infoset)]; - if (auto parent_sp = component_top_node->GetParent()) { - auto *parent = parent_sp.get(); - if (data.node_visited_token[parent] == 0) { - global_frontier.push(parent); - data.node_visited_token[parent] = data.current_token; - } - } } std::map result; From dd47f40b2e5429af9bea64c84dd9b7189aa16d42 Mon Sep 17 00:00:00 2001 From: drdkad Date: Mon, 8 Dec 2025 08:12:29 +0000 Subject: [PATCH 09/18] Consolidate two subgame root tests into a single path-based verification --- tests/test_node.py | 57 ++++++++-------------------------------------- 1 file changed, 9 insertions(+), 48 deletions(-) diff --git a/tests/test_node.py b/tests/test_node.py index bc78577a7..c296b7e60 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -199,43 +199,8 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): # ============================================================================== -# New Test Suite for the Subgame Root Checker +# Test Suite for the Subgame Root Checker # ============================================================================== - -@pytest.mark.parametrize("game, expected_result", [ - # Games without Absent-Mindedness for which the legacy method is known to be correct. - (games.read_from_file("wichardt.efg"), {0}), - (games.read_from_file("e02.efg"), {0, 2, 4}), - (games.read_from_file("subgame-roots-finder-one-merge.efg"), {0, 2}), - ( - games.read_from_file("subgame-roots-finder-multiple-roots-and-merge.efg"), - {0, 1, 4, 7, 11, 13, 34} - ), - (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], {0, 2, 4, 6, 8}), - (gbt.Game.new_tree(), {}), - (games.read_from_file("subgame-roots-finder-multiple-merges.efg"), {0, 2, 13, 15}), - - # Games with Absent-Mindedness where the legacy method is known to fail. - pytest.param( - games.read_from_file("AM-subgames.efg"), - {0, 2, 5, 8}, # The correct set of roots - marks=pytest.mark.xfail( - reason="Legacy method does not detect subgame roots that are members of AM-infosets." - ) - ), -]) -def test_legacy_is_subgame_root_set(game: gbt.Game, expected_result: set): - """ - Tests the legacy `node.is_subgame_root` against an expected set of nodes. - Includes both passing cases and games with Absent-Mindedness where it is expected to fail. - """ - list_nodes = list(game.nodes) - expected_roots = {list_nodes[i] for i in expected_result} - legacy_roots = {node for node in game.nodes if node.is_subgame_root} - assert legacy_roots == expected_roots - - -# NEW subgame finder algorithm @pytest.mark.parametrize("game, expected_paths_list", [ # Empty game has no subgames ( @@ -323,21 +288,17 @@ def test_legacy_is_subgame_root_set(game: gbt.Game, expected_result: set): ] ), ]) -def test_new_subgame_root_method_paths(game: gbt.Game, expected_paths_list: list): +def test_subgame_root_consistency(game: gbt.Game, expected_paths_list: list): """ - Tests `game.subgame_root()` by comparing the paths of the identified root nodes - against the expected paths. + Tests `game.subgame_root` and `node.is_subgame_root` for consistency and correctness + by comparing the paths of the identified root nodes against the expected paths. """ - actual_root_nodes = { - game.subgame_root(infoset) - for infoset in game.infosets - if not infoset.is_chance - } + roots_from_lookup = {game.subgame_root(infoset) for infoset in game.infosets} + roots_from_property = {node for node in game.nodes if node.is_subgame_root} - actual_paths = [ - _get_path_of_action_labels(node) - for node in actual_root_nodes - ] + assert roots_from_lookup == roots_from_property + + actual_paths = [_get_path_of_action_labels(node) for node in roots_from_lookup] assert sorted(actual_paths) == sorted(expected_paths_list) From aee924db5e2dd3c0f9c2b050d47b98d58a0a1569 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 11 Dec 2025 18:01:56 +0000 Subject: [PATCH 10/18] Refactor the local frontier to be a priority queue --- src/games/gametree.cc | 82 ++++++++++++++++++++----------------------- 1 file changed, 39 insertions(+), 43 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 784a08edc..63e696100 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -28,6 +28,7 @@ #include #include #include +#include #include "gambit.h" #include "gametree.h" @@ -1077,7 +1078,15 @@ void GameTreeRep::BuildUnreachableNodes() const // Subgame Root Finder //------------------------------------------------------------------------ -namespace { // Anonymous implementation namespace +namespace { // Anonymous namespace + +// Comparator for priority queue (ancestors have lower priority than descendants) +struct NodeCmp { + bool operator()(const GameNodeRep *a, const GameNodeRep *b) const + { + return a->GetNumber() < b->GetNumber(); + } +}; // Scratch data structure for subgame root finding algorithm struct SubgameScratchData { @@ -1088,20 +1097,18 @@ struct SubgameScratchData { std::vector node_visited_token; int current_token = 0; - std::vector local_frontier; - // DSU Find with path compression GameInfosetRep *FindSet(GameInfosetRep *p_infoset) { // Initialize/retrieve current parent auto *leader = dsu_parent.try_emplace(p_infoset, p_infoset).first->second; - while (dsu_parent[leader] != leader) { - leader = dsu_parent[leader]; + while (dsu_parent.at(leader) != leader) { + leader = dsu_parent.at(leader); } // Path compression auto *curr = p_infoset; while (curr != leader) { - auto &parent_ref = dsu_parent[curr]; + auto &parent_ref = dsu_parent.at(curr); curr = parent_ref; parent_ref = leader; } @@ -1120,8 +1127,11 @@ struct SubgameScratchData { return; } - auto *existing_candidate = subgame_root_candidate[leader_current]; + // Check if candidate exists before accessing + auto it = subgame_root_candidate.find(leader_current); + auto *existing_candidate = (it != subgame_root_candidate.end()) ? it->second : nullptr; auto *best_candidate = existing_candidate ? existing_candidate : p_current_node; + dsu_parent[leader_current] = leader_start; subgame_root_candidate[leader_start] = best_candidate; } @@ -1133,31 +1143,14 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) auto *start_infoset = p_start_node->GetInfoset().get(); p_data.current_token++; - auto &frontier = p_data.local_frontier; - size_t active_nodes_counter = 0; + std::priority_queue, NodeCmp> local_frontier; - const int max_node_number = p_start_node->GetNumber(); - int min_node_number = p_start_node->GetNumber(); // is dynamically updated as the frontier grows + local_frontier.push(p_start_node); + p_data.node_visited_token[p_start_node->GetNumber()] = p_data.current_token; - // Add node to frontier and track minimum - auto push_node = [&](GameNodeRep *node) { - const int node_num = node->GetNumber(); - min_node_number = std::min(min_node_number, node_num); - frontier[node_num] = node; - p_data.node_visited_token[node_num] = p_data.current_token; - active_nodes_counter++; - }; - - push_node(p_start_node); - - // Process nodes in descending order - for (int node_num = max_node_number; node_num >= min_node_number; --node_num) { - auto *curr = frontier[node_num]; - if (!curr) { - continue; // Skip empty slots - } - - active_nodes_counter--; + while (!local_frontier.empty()) { + auto *curr = local_frontier.top(); + local_frontier.pop(); auto *curr_infoset = curr->GetInfoset().get(); const bool is_external_collision = p_data.dsu_parent.count(curr_infoset); @@ -1165,29 +1158,34 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) p_data.UnionSets(start_infoset, curr_infoset, curr); if (is_external_collision) { - auto *candidate_root = p_data.subgame_root_candidate[p_data.FindSet(curr_infoset)]; + // We hit a node belonging to a previously generated component. + auto *candidate_root = p_data.subgame_root_candidate.at(p_data.FindSet(curr_infoset)); if (p_data.node_visited_token[candidate_root->GetNumber()] != p_data.current_token) { - push_node(candidate_root); + local_frontier.push(candidate_root); + p_data.node_visited_token[candidate_root->GetNumber()] = p_data.current_token; } } else { + // First time seeing this infoset: add all its members to the frontier. for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { auto *member = member_sp.get(); if (member == curr) { continue; } - push_node(member); + local_frontier.push(member); + p_data.node_visited_token[member->GetNumber()] = p_data.current_token; } } - if (active_nodes_counter > 0) { - auto *parent = curr->GetParent().get(); - if (p_data.node_visited_token[parent->GetNumber()] != p_data.current_token) { - push_node(parent); + if (!local_frontier.empty()) { + if (auto parent_sp = curr->GetParent()) { + auto *parent = parent_sp.get(); + if (p_data.node_visited_token[parent->GetNumber()] != p_data.current_token) { + local_frontier.push(parent); + p_data.node_visited_token[parent->GetNumber()] = p_data.current_token; + } } } - - frontier[node_num] = nullptr; } } @@ -1202,11 +1200,9 @@ std::map FindSubgameRoots(const Game &p_game) data.current_token++; const int num_nodes = p_game->NumNodes(); - // Allocate local frontier once for reuse across all component generations - data.local_frontier.resize(num_nodes + 1, nullptr); data.node_visited_token.resize(num_nodes + 1, 0); - // Collect nodes in ascending order (depth-first traversal) + // Collect nodes in ascending order (depth-first traversal) -- TODO: Replace with proper iterator std::vector nodes_ascending; nodes_ascending.reserve(num_nodes); for (const auto &node : p_game->GetNodes()) { @@ -1227,7 +1223,7 @@ std::map FindSubgameRoots(const Game &p_game) auto collect_results = [&](const GamePlayer &p_player) { for (const auto &infoset : p_player->GetInfosets()) { auto *ptr = infoset.get(); - result[ptr] = data.subgame_root_candidate[data.FindSet(ptr)]; + result[ptr] = data.subgame_root_candidate.at(data.FindSet(ptr)); } }; From eeb32ea0b904a0c33f26a99fc0894e6ad74a7089 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 11 Dec 2025 20:46:43 +0000 Subject: [PATCH 11/18] Tests updated to reflect trivial game edge case. --- tests/test_node.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/test_node.py b/tests/test_node.py index c296b7e60..91b324d65 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -205,7 +205,7 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): # Empty game has no subgames ( gbt.Game.new_tree(), - [] + [[]] ), # --- Games without Absent-Mindedness. --- # Perfect Information @@ -293,14 +293,18 @@ def test_subgame_root_consistency(game: gbt.Game, expected_paths_list: list): Tests `game.subgame_root` and `node.is_subgame_root` for consistency and correctness by comparing the paths of the identified root nodes against the expected paths. """ - roots_from_lookup = {game.subgame_root(infoset) for infoset in game.infosets} roots_from_property = {node for node in game.nodes if node.is_subgame_root} - assert roots_from_lookup == roots_from_property + # For trivial games with no infosets, check the property-based roots + if len(game.infosets) == 0: + actual_paths = [_get_path_of_action_labels(node) for node in roots_from_property] + assert actual_paths == expected_paths_list + else: + roots_from_lookup = {game.subgame_root(infoset) for infoset in game.infosets} + assert roots_from_lookup == roots_from_property - actual_paths = [_get_path_of_action_labels(node) for node in roots_from_lookup] - - assert sorted(actual_paths) == sorted(expected_paths_list) + actual_paths = [_get_path_of_action_labels(node) for node in roots_from_lookup] + assert sorted(actual_paths) == sorted(expected_paths_list) @pytest.mark.parametrize("game_file, expected_unreachable_paths", [ From 8beba493f868ba584da8554b9d3ddf3d5fff4dc5 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 11 Dec 2025 20:53:49 +0000 Subject: [PATCH 12/18] Raise UndefinedException in virtual GetSubgameRoot() in the base class; remove overrides from other child classes --- src/games/game.h | 2 +- src/games/gameagg.h | 1 - src/games/gamebagg.h | 1 - src/games/gametable.h | 1 - 4 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index 99d860edc..06a13850f 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -837,7 +837,7 @@ class GameRep : public std::enable_shared_from_this { return false; } /// - virtual GameNode GetSubgameRoot(const GameInfoset &) const = 0; + virtual GameNode GetSubgameRoot(const GameInfoset &) const { throw UndefinedException(); } //@} /// @name Writing data files diff --git a/src/games/gameagg.h b/src/games/gameagg.h index f16d6c97e..bd12f7da7 100644 --- a/src/games/gameagg.h +++ b/src/games/gameagg.h @@ -81,7 +81,6 @@ class GameAGGRep : public GameRep { bool IsTree() const override { return false; } bool IsAgg() const override { return true; } bool IsPerfectRecall() const override { return true; } - GameNode GetSubgameRoot(const GameInfoset &) const override { return {nullptr}; } bool IsConstSum() const override; /// Returns the smallest payoff to any player in any outcome of the game Rational GetMinPayoff() const override { return Rational(aggPtr->getMinPayoff()); } diff --git a/src/games/gamebagg.h b/src/games/gamebagg.h index 9d6cc4680..3918a58d2 100644 --- a/src/games/gamebagg.h +++ b/src/games/gamebagg.h @@ -88,7 +88,6 @@ class GameBAGGRep : public GameRep { bool IsTree() const override { return false; } virtual bool IsBagg() const { return true; } bool IsPerfectRecall() const override { return true; } - GameNode GetSubgameRoot(const GameInfoset &) const override { return {nullptr}; } bool IsConstSum() const override { throw UndefinedException(); } /// Returns the smallest payoff to any player in any outcome of the game Rational GetMinPayoff() const override { return Rational(baggPtr->getMinPayoff()); } diff --git a/src/games/gametable.h b/src/games/gametable.h index 445b4c443..3aab1dd38 100644 --- a/src/games/gametable.h +++ b/src/games/gametable.h @@ -64,7 +64,6 @@ class GameTableRep : public GameExplicitRep { Rational GetPlayerMaxPayoff(const GamePlayer &) const override; bool IsPerfectRecall() const override { return true; } - GameNode GetSubgameRoot(const GameInfoset &) const override { return {nullptr}; } //@} /// @name Dimensions of the game From efe95bc4f7cfbf4a5bf13bfb9a7d410c2cdfee93 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 11 Dec 2025 20:57:22 +0000 Subject: [PATCH 13/18] Replace token-based visited tracking with std::unordered_set; correct edge case of the empty game; add documentation --- src/games/gametree.cc | 84 ++++++++++++++++++++++++++++++++----------- 1 file changed, 64 insertions(+), 20 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 63e696100..a05a0a032 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -374,7 +374,7 @@ bool GameNodeRep::IsSuccessorOf(GameNode p_node) const bool GameNodeRep::IsSubgameRoot() const { if (m_children.empty()) { - return false; + return !GetParent(); } return m_game->GetSubgameRoot(m_infoset->shared_from_this()) == shared_from_this(); } @@ -1081,6 +1081,8 @@ void GameTreeRep::BuildUnreachableNodes() const namespace { // Anonymous namespace // Comparator for priority queue (ancestors have lower priority than descendants) +// This ensures descendants are processed first, allowing upward traversal to find +// the highest reachable node in each component struct NodeCmp { bool operator()(const GameNodeRep *a, const GameNodeRep *b) const { @@ -1088,16 +1090,21 @@ struct NodeCmp { } }; -// Scratch data structure for subgame root finding algorithm struct SubgameScratchData { - // DSU structures + /// DSU structures + /// + /// Maps each infoset to its parent in the union-find structure. + /// After path compression, points to the component leader directly. std::unordered_map dsu_parent; + /// Maps each component leader to the highest node (candidate root) in that component std::unordered_map subgame_root_candidate; - // Timestamps for nodes - std::vector node_visited_token; - int current_token = 0; - // DSU Find with path compression + /// DSU Find + path compression: Finds the leader of the component containing p_infoset. + /// + /// @param p_infoset The infoset to find the component leader for + /// @return Pointer to the leader infoset of the component + /// + /// Postcondition: Path from p_infoset to leader is compressed (flattened) GameInfosetRep *FindSet(GameInfosetRep *p_infoset) { // Initialize/retrieve current parent @@ -1115,7 +1122,14 @@ struct SubgameScratchData { return leader; } - // DSU Union - merge current infoset into the start one and update the root candidate + /// DSU Union: Merges the components containing two infosets, updates the subgame root candidate. + /// + /// @param p_start_infoset The infoset whose component should absorb the other + /// @param p_current_infoset The infoset whose component is being merged + /// @param p_current_node The node being processed, considered as potential subgame root + /// + /// Postcondition: Both infosets belong to the same component + /// Postcondition: The component's subgame_root_candidate is updated to the highest node void UnionSets(GameInfosetRep *p_start_infoset, GameInfosetRep *p_current_infoset, GameNodeRep *p_current_node) { @@ -1137,16 +1151,37 @@ struct SubgameScratchData { } }; -// Generate a single component starting from a given node +/// Generates a single connected component starting from a given node. +/// +/// The local frontier driving the exploration is a priority queue. +/// This design choice ensures that nodes are processed before their ancestors. +/// +/// Starting from p_start_node, explores the game tree by: +/// 1. Adding all members of each encountered infoset (horizontal expansion of the frontier) +/// 2. Moving to parent nodes (vertical expansion of the frontier) +/// 3. When hitting nodes from previously-generated components (external collision) +/// it merges the components and adds the root of the external component to the frontier +/// +/// The component gathers all infosets that share the same minimal subgame root. +/// The highest node processed that empties the frontier without adding any new nodes +/// to horizontal expansion becomes the subgame root candidate. +/// +/// @param p_data The DSU data structure to update with component information +/// @param p_start_node The node to start component generation from +/// +/// Precondition: p_start_node must be non-terminal +/// Precondition: p_start_node's infoset must not already be in p_data.dsu_parent +/// Postcondition: p_data.subgame_root_candidate[leader] contains the highest node +/// in the newly-formed component void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) { auto *start_infoset = p_start_node->GetInfoset().get(); - p_data.current_token++; + std::unordered_set visited_this_component; std::priority_queue, NodeCmp> local_frontier; local_frontier.push(p_start_node); - p_data.node_visited_token[p_start_node->GetNumber()] = p_data.current_token; + visited_this_component.insert(p_start_node); while (!local_frontier.empty()) { auto *curr = local_frontier.top(); @@ -1160,9 +1195,9 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) if (is_external_collision) { // We hit a node belonging to a previously generated component. auto *candidate_root = p_data.subgame_root_candidate.at(p_data.FindSet(curr_infoset)); - if (p_data.node_visited_token[candidate_root->GetNumber()] != p_data.current_token) { + if (!visited_this_component.count(candidate_root)) { local_frontier.push(candidate_root); - p_data.node_visited_token[candidate_root->GetNumber()] = p_data.current_token; + visited_this_component.insert(candidate_root); } } else { @@ -1173,23 +1208,35 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) continue; } local_frontier.push(member); - p_data.node_visited_token[member->GetNumber()] = p_data.current_token; + visited_this_component.insert(member); } } if (!local_frontier.empty()) { if (auto parent_sp = curr->GetParent()) { auto *parent = parent_sp.get(); - if (p_data.node_visited_token[parent->GetNumber()] != p_data.current_token) { + if (!visited_this_component.count(parent)) { local_frontier.push(parent); - p_data.node_visited_token[parent->GetNumber()] = p_data.current_token; + visited_this_component.insert(parent); } } } } } -// For each infoset, find the root of the smallest subgame containing it +/// For each infoset in the game, computes the root of the smallest subgame containing it. +/// +/// Processes nodes in reverse DFS order (postorder), building a component for each node, +/// skipping a node if it is: +/// 1. A member of an infoset already belonging to some component, or +/// 2. Terminal +/// +/// @param p_game The game tree +/// @return Map from each infoset to the root of its smallest containing subgame +/// +/// Precondition: p_game must be a valid game tree +/// Postcondition: Every infoset in the game is mapped to exactly one subgame root node +/// Returns: Empty map if the game root is terminal (trivial game) std::map FindSubgameRoots(const Game &p_game) { if (p_game->GetRoot()->IsTerminal()) { @@ -1197,11 +1244,8 @@ std::map FindSubgameRoots(const Game &p_game) } SubgameScratchData data; - data.current_token++; const int num_nodes = p_game->NumNodes(); - data.node_visited_token.resize(num_nodes + 1, 0); - // Collect nodes in ascending order (depth-first traversal) -- TODO: Replace with proper iterator std::vector nodes_ascending; nodes_ascending.reserve(num_nodes); From 599f41c9d1cfe4aef8494cf376e98b94a13ea2b7 Mon Sep 17 00:00:00 2001 From: drdkad Date: Tue, 16 Dec 2025 07:05:46 +0000 Subject: [PATCH 14/18] Use the post-order iterator in FindSubgameRoots --- src/games/gametree.cc | 15 +++------------ src/games/gametree.h | 2 +- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index a05a0a032..ccac60256 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1244,22 +1244,13 @@ std::map FindSubgameRoots(const Game &p_game) } SubgameScratchData data; - const int num_nodes = p_game->NumNodes(); - // Collect nodes in ascending order (depth-first traversal) -- TODO: Replace with proper iterator - std::vector nodes_ascending; - nodes_ascending.reserve(num_nodes); - for (const auto &node : p_game->GetNodes()) { - nodes_ascending.push_back(node.get()); - } - - // Process in reverse order (descending node numbers, children before parents) - for (auto it = nodes_ascending.rbegin(); it != nodes_ascending.rend(); ++it) { - auto *node = *it; + // Process nodes in postorder + for (const auto &node : p_game->GetNodes(TraversalOrder::Postorder)) { if (node->IsTerminal() || data.dsu_parent.count(node->GetInfoset().get())) { continue; } - GenerateComponent(data, node); + GenerateComponent(data, node.get()); } std::map result; diff --git a/src/games/gametree.h b/src/games/gametree.h index 2b2d34f65..75c99b59b 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -89,7 +89,7 @@ class GameTreeRep : public GameExplicitRep { /// Returns the largest payoff to the player in any play of the game Rational GetPlayerMaxPayoff(const GamePlayer &) const override; bool IsAbsentMinded(const GameInfoset &p_infoset) const override; - GameNode GetSubgameRoot(GameInfoset infoset) const override + GameNode GetSubgameRoot(const GameInfoset &infoset) const override { if (m_infosetSubgameRoot.empty()) { const_cast(this)->BuildSubgameRoots(); From 9d68d1f081b47629fc0331469b1f9a08606ba3ea Mon Sep 17 00:00:00 2001 From: drdkad Date: Wed, 17 Dec 2025 11:31:54 +0000 Subject: [PATCH 15/18] Tightened tests --- tests/test_node.py | 97 +++++++++------------------------------------- 1 file changed, 18 insertions(+), 79 deletions(-) diff --git a/tests/test_node.py b/tests/test_node.py index 91b324d65..384edfdea 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -202,91 +202,30 @@ def test_node_own_prior_action_non_terminal(game_file, expected_node_data): # Test Suite for the Subgame Root Checker # ============================================================================== @pytest.mark.parametrize("game, expected_paths_list", [ - # Empty game has no subgames - ( - gbt.Game.new_tree(), - [[]] - ), + # Empty game + (gbt.Game.new_tree(), [[]]), + # --- Games without Absent-Mindedness. --- # Perfect Information - ( - games.read_from_file("e02.efg"), - [ - [], - ["L"], - ["L", "L"] - ] - ), - ( - games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], - [ - ["Push", "Push"], - ["Push", "Push", "Push", "Push"], - ["Push", "Push", "Push"], - ["Push"], - [] - ] - ), + (games.read_from_file("e02.efg"), [[], ["L"], ["L", "L"]]), + (games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], + [["Push", "Push"], ["Push", "Push", "Push", "Push"], ["Push", "Push", "Push"], ["Push"], []]), + # Perfect Recall - ( - games.read_from_file("binary_3_levels_generic_payoffs.efg"), - [ - [] - ] - ), + (games.read_from_file("binary_3_levels_generic_payoffs.efg"), [[]]), + # No perfect recall - ( - games.read_from_file("wichardt.efg"), - [ - [] - ] - ), - ( - games.read_from_file("subgame-roots-finder-one-merge.efg"), - [ - [], - ["1"] - ] - ), - ( - games.read_from_file("subgame-roots-finder-small-subgames-and-merges.efg"), - [ - ["2"], - ["1"], - ["1", "2", "2"], - ["2", "1", "2"], - [], - ["1", "1", "1", "2", "2"], - ["2", "2", "2"] - ] - ), - ( - games.read_from_file("subgame-roots-finder-multiple-merges.efg"), - [ - [], - ["1", "1"], - ["1"], - ["1", "1", "1"] - ] - ), + (games.read_from_file("wichardt.efg"), [[]]), + (games.read_from_file("subgame-roots-finder-one-merge.efg"), [[], ["1"]]), + (games.read_from_file("subgame-roots-finder-small-subgames-and-merges.efg"), + [["2"], ["1"], ["1", "2", "2"], ["2", "1", "2"], [], + ["1", "1", "1", "2", "2"], ["2", "2", "2"]]), + (games.read_from_file("subgame-roots-finder-multiple-merges.efg"), + [[], ["1", "1"], ["1"], ["1", "1", "1"]]), # --- Games with Absent-Mindedness. --- - ( - games.read_from_file("AM-subgames.efg"), - [ - [], - ["1", "1"], - ["2"], - ["2", "1"] - ] - ), - ( - games.read_from_file("noPR-action-AM-two-hops.efg"), - [ - [], - ["2", "1", "1"] - ] - ), + (games.read_from_file("AM-subgames.efg"), [[], ["1", "1"], ["2"], ["2", "1"]]), + (games.read_from_file("noPR-action-AM-two-hops.efg"), [[], ["2", "1", "1"]]), ]) def test_subgame_root_consistency(game: gbt.Game, expected_paths_list: list): """ From 4f3033d34ffa840c3279bc4f599927de9fffcef8 Mon Sep 17 00:00:00 2001 From: drdkad Date: Wed, 17 Dec 2025 11:35:54 +0000 Subject: [PATCH 16/18] Apply filter_if, GetPlayersWithChance and GetNonterminalNodes tightenings --- src/games/gametree.cc | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index ccac60256..70f01bfbe 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1202,11 +1202,9 @@ void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) } else { // First time seeing this infoset: add all its members to the frontier. - for (const auto &member_sp : curr->GetInfoset()->GetMembers()) { + for (const auto &member_sp : filter_if(curr->GetInfoset()->GetMembers(), + [curr](const auto &m) { return m.get() != curr; })) { auto *member = member_sp.get(); - if (member == curr) { - continue; - } local_frontier.push(member); visited_this_component.insert(member); } @@ -1246,8 +1244,8 @@ std::map FindSubgameRoots(const Game &p_game) SubgameScratchData data; // Process nodes in postorder - for (const auto &node : p_game->GetNodes(TraversalOrder::Postorder)) { - if (node->IsTerminal() || data.dsu_parent.count(node->GetInfoset().get())) { + for (const auto &node : p_game->GetNonterminalNodes(TraversalOrder::Postorder)) { + if (data.dsu_parent.count(node->GetInfoset().get())) { continue; } GenerateComponent(data, node.get()); @@ -1255,16 +1253,11 @@ std::map FindSubgameRoots(const Game &p_game) std::map result; - auto collect_results = [&](const GamePlayer &p_player) { - for (const auto &infoset : p_player->GetInfosets()) { + for (const auto &player : p_game->GetPlayersWithChance()) { + for (const auto &infoset : player->GetInfosets()) { auto *ptr = infoset.get(); result[ptr] = data.subgame_root_candidate.at(data.FindSet(ptr)); } - }; - - collect_results(p_game->GetChance()); - for (const auto &player : p_game->GetPlayers()) { - collect_results(player); } return result; From 1216beca25aa89498f4afc3ca979e0ed48bca411 Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 18 Dec 2025 11:59:24 +0000 Subject: [PATCH 17/18] Refactor subgame root finder with NestedElementCollection and filter_if --- src/games/gametree.cc | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 70f01bfbe..6787e5f66 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1079,17 +1079,6 @@ void GameTreeRep::BuildUnreachableNodes() const //------------------------------------------------------------------------ namespace { // Anonymous namespace - -// Comparator for priority queue (ancestors have lower priority than descendants) -// This ensures descendants are processed first, allowing upward traversal to find -// the highest reachable node in each component -struct NodeCmp { - bool operator()(const GameNodeRep *a, const GameNodeRep *b) const - { - return a->GetNumber() < b->GetNumber(); - } -}; - struct SubgameScratchData { /// DSU structures /// @@ -1175,13 +1164,17 @@ struct SubgameScratchData { /// in the newly-formed component void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) { - auto *start_infoset = p_start_node->GetInfoset().get(); + auto node_cmp = [](const GameNodeRep *a, const GameNodeRep *b) { + return a->GetNumber() < b->GetNumber(); + }; + std::priority_queue, decltype(node_cmp)> + local_frontier(node_cmp); std::unordered_set visited_this_component; - std::priority_queue, NodeCmp> local_frontier; local_frontier.push(p_start_node); visited_this_component.insert(p_start_node); + auto *start_infoset = p_start_node->GetInfoset().get(); while (!local_frontier.empty()) { auto *curr = local_frontier.top(); @@ -1244,20 +1237,21 @@ std::map FindSubgameRoots(const Game &p_game) SubgameScratchData data; // Process nodes in postorder - for (const auto &node : p_game->GetNonterminalNodes(TraversalOrder::Postorder)) { - if (data.dsu_parent.count(node->GetInfoset().get())) { - continue; - } + for (const auto &node : filter_if(p_game->GetNonterminalNodes(TraversalOrder::Postorder), + [&data](const auto &node) { + return !data.dsu_parent.count(node->GetInfoset().get()); + })) { GenerateComponent(data, node.get()); } std::map result; - for (const auto &player : p_game->GetPlayersWithChance()) { - for (const auto &infoset : player->GetInfosets()) { - auto *ptr = infoset.get(); - result[ptr] = data.subgame_root_candidate.at(data.FindSet(ptr)); - } + using InfosetsWithChance = + NestedElementCollection; + + for (const auto &infoset : InfosetsWithChance(p_game)) { + auto *ptr = infoset.get(); + result[ptr] = data.subgame_root_candidate.at(data.FindSet(ptr)); } return result; @@ -1267,9 +1261,6 @@ std::map FindSubgameRoots(const Game &p_game) void GameTreeRep::BuildSubgameRoots() const { - if (!m_computedValues) { - BuildComputedValues(); - } m_infosetSubgameRoot = FindSubgameRoots(std::const_pointer_cast(shared_from_this())); } From f6e1f68e3e6408414295372c7fb7a99a0850eafc Mon Sep 17 00:00:00 2001 From: drdkad Date: Thu, 18 Dec 2025 12:32:33 +0000 Subject: [PATCH 18/18] Pre-compute DFS numbers locally within FindSubgameRoots() to eliminate dependency on SortInfosets() --- src/games/gametree.cc | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 6787e5f66..17d70123c 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1157,16 +1157,19 @@ struct SubgameScratchData { /// /// @param p_data The DSU data structure to update with component information /// @param p_start_node The node to start component generation from +/// @param p_node_ordering A map providing a strict total ordering (DFS Preorder) of nodes /// /// Precondition: p_start_node must be non-terminal /// Precondition: p_start_node's infoset must not already be in p_data.dsu_parent /// Postcondition: p_data.subgame_root_candidate[leader] contains the highest node /// in the newly-formed component -void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node) +void GenerateComponent(SubgameScratchData &p_data, GameNodeRep *p_start_node, + const std::unordered_map &p_node_ordering) { - auto node_cmp = [](const GameNodeRep *a, const GameNodeRep *b) { - return a->GetNumber() < b->GetNumber(); + auto node_cmp = [&p_node_ordering](const GameNodeRep *a, const GameNodeRep *b) { + return p_node_ordering.at(a) < p_node_ordering.at(b); }; + std::priority_queue, decltype(node_cmp)> local_frontier(node_cmp); @@ -1234,14 +1237,24 @@ std::map FindSubgameRoots(const Game &p_game) return {}; } + // Pre-compute DFS numbers locally. + std::unordered_map node_ordering; + int counter = 0; + for (const auto &node : p_game->GetNodes(TraversalOrder::Preorder)) { + node_ordering[node.get()] = counter++; + } + SubgameScratchData data; + // Define filter predicate + auto is_unvisited_infoset = [&data](const auto &node) { + return !data.dsu_parent.count(node->GetInfoset().get()); + }; + // Process nodes in postorder - for (const auto &node : filter_if(p_game->GetNonterminalNodes(TraversalOrder::Postorder), - [&data](const auto &node) { - return !data.dsu_parent.count(node->GetInfoset().get()); - })) { - GenerateComponent(data, node.get()); + for (const auto &node : + filter_if(p_game->GetNonterminalNodes(TraversalOrder::Postorder), is_unvisited_infoset)) { + GenerateComponent(data, node.get(), node_ordering); } std::map result;