From 05230c75fb78af43d8cdad5845097e928984130e Mon Sep 17 00:00:00 2001 From: drdkad Date: Sun, 1 Feb 2026 10:23:11 +0000 Subject: [PATCH 01/10] Refactor subgame roots tests to use factory pattern and dataclasses. Add new test games --- tests/test_games/AM-subgames.efg | 14 ++ ...nder_overplapping_infosets_with_Nature.efg | 55 ++++++++ ...all_subgames_and_overplapping_infosets.efg | 48 +++++++ tests/test_node.py | 131 ++++++++++++++---- 4 files changed, 222 insertions(+), 26 deletions(-) create mode 100644 tests/test_games/AM-subgames.efg create mode 100644 tests/test_games/subgame_roots_finder_overplapping_infosets_with_Nature.efg create mode 100644 tests/test_games/subgame_roots_finder_small_subgames_and_overplapping_infosets.efg 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_overplapping_infosets_with_Nature.efg b/tests/test_games/subgame_roots_finder_overplapping_infosets_with_Nature.efg new file mode 100644 index 000000000..bb6c6b0f0 --- /dev/null +++ b/tests/test_games/subgame_roots_finder_overplapping_infosets_with_Nature.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_small_subgames_and_overplapping_infosets.efg b/tests/test_games/subgame_roots_finder_small_subgames_and_overplapping_infosets.efg new file mode 100644 index 000000000..e54ada511 --- /dev/null +++ b/tests/test_games/subgame_roots_finder_small_subgames_and_overplapping_infosets.efg @@ -0,0 +1,48 @@ +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 +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 5 "" { "1" "2" } 0 +p "" 1 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 7 "" { "1" "2" } 0 +t "" 9 "Outcome 9" { 9, -9 } +t "" 10 "Outcome 10" { 10, -10 } +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 } +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 6 "" { "1" "2" } 0 +t "" 16 "Outcome 16" { 16, -16 } +t "" 17 "Outcome 17" { 17, -17 } +p "" 1 7 "" { "1" "2" } 0 +t "" 18 "Outcome 18" { 18, -18 } +t "" 19 "Outcome 19" { 19, -19 } diff --git a/tests/test_node.py b/tests/test_node.py index 012e9ce0a..c3d35d04f 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -1,3 +1,5 @@ +import dataclasses +import functools import itertools import typing @@ -93,32 +95,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. @@ -136,6 +112,109 @@ def _get_path_of_action_labels(node: gbt.Node) -> list[str]: return path +@dataclasses.dataclass +class SubgameRootsTestCase: + """TestCase for testing subgame root detection.""" + factory: typing.Callable[[], gbt.Game] + expected_paths: list[list[str]] + + +SUBGAME_ROOTS_CASES = [ + # ------------------------------------------------------------------------ + # Empty Game + # ------------------------------------------------------------------------ + pytest.param( + SubgameRootsTestCase(factory=gbt.Game.new_tree, expected_paths=[[]]), + id="empty_tree" + ), + # ------------------------------------------------------------------------ + # Perfect Information Games + # ------------------------------------------------------------------------ + pytest.param( + SubgameRootsTestCase( + factory=functools.partial(games.read_from_file, "e02.efg"), + expected_paths=[[], ["L"], ["L", "L"]] + ), + id="centipede_3_rounds" + ), + pytest.param( + SubgameRootsTestCase( + factory=lambda: games.Centipede.get_test_data(N=5, m0=2, m1=7)[0], + expected_paths=[[], ["Push"], ["Push", "Push"], ["Push", "Push", "Push"], + ["Push", "Push", "Push", "Push"]] + ), + id="centipede_5_rounds" + ), + # ------------------------------------------------------------------------ + # Imperfect Information (No Absent-Mindedness) + # ------------------------------------------------------------------------ + pytest.param( + SubgameRootsTestCase( + factory=functools.partial(games.read_from_file, "wichardt.efg"), + expected_paths=[[]] + ), + id="wichardt_no_nontrivial_subgames" + ), + pytest.param( + SubgameRootsTestCase( + factory=functools.partial(games.read_from_file, "binary_3_levels_generic_payoffs.efg"), + expected_paths=[[]] + ), + id="binary_3_levels_no_nontrivial_subgames" + ), + pytest.param( + SubgameRootsTestCase( + factory=functools.partial( + games.read_from_file, + "subgame_roots_finder_small_subgames_and_overplapping_infosets.efg"), + expected_paths=[[], ["1"], ["2"], ["1", "2", "2"], ["2", "1", "2"], + ["1", "1", "1", "2", "2"], ["2", "2", "2"]] + ), + id="small_subgames_and_overlapping_infosets_inside_subgames_no_Nature_moves" + ), + pytest.param( + SubgameRootsTestCase( + factory=functools.partial( + games.read_from_file, + "subgame_roots_finder_overplapping_infosets_with_Nature.efg"), + expected_paths=[[], ["1"], ["1", "1"], ["1", "1", "1"]] + ), + id="overlapping_infosets_inside_subgames_and_Nature_move" + ), + # ------------------------------------------------------------------------ + # Absent-Minded Games + # ------------------------------------------------------------------------ + pytest.param( + SubgameRootsTestCase( + factory=functools.partial(games.read_from_file, "AM-subgames.efg"), + expected_paths=[[], ["2"], ["1", "1"], ["2", "1"]] + ), + id="Absent-minded-game-with-paths-intersecting-infoset-two-times" + ), + pytest.param( + SubgameRootsTestCase( + factory=functools.partial(games.read_from_file, "noPR-action-AM-two-hops.efg"), + expected_paths=[[], ["2", "1", "1"]] + ), + id="Absent-minded-game-with-paths-intersecting-infoset-three-times" + ), +] + + +@pytest.mark.parametrize("test_case", SUBGAME_ROOTS_CASES) +def test_subgame_roots(test_case: SubgameRootsTestCase): + """ + Tests that the set of nodes marked as subgame roots matches the expected + set of paths (Action Labels from Root -> Node). + """ + game = test_case.factory() + + actual_roots = [node for node in game.nodes if node.is_subgame_root] + actual_paths = [_get_path_of_action_labels(node) for node in actual_roots] + + assert sorted(actual_paths) == sorted(test_case.expected_paths) + + @pytest.mark.parametrize("game_file, expected_node_data", [ ( "binary_3_levels_generic_payoffs.efg", From 7cf4bf046eeb3e4e3c64633b8ae196f55052ffcf Mon Sep 17 00:00:00 2001 From: drdkad Date: Sun, 1 Feb 2026 10:26:59 +0000 Subject: [PATCH 02/10] Implement subgame root detection via interval reachability. Add BuildSubgameRoots() using a Tarjan-style algorithm adapted for game trees. Update IsSubgameRoot() to use cached results with lazy computation. --- src/games/game.h | 3 + src/games/gametree.cc | 185 ++++++++++++++++++++++++++++++++++++------ src/games/gametree.h | 3 + 3 files changed, 167 insertions(+), 24 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index f3df5b2dc..4e85f9060 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -924,6 +924,9 @@ class GameRep : public std::enable_shared_from_this { } return false; } + /// Returns a list of all subgame roots in the game + virtual std::vector GetSubgames() const { throw UndefinedException(); } + //@} /// @name Writing data files diff --git a/src/games/gametree.cc b/src/games/gametree.cc index e3e6b656e..ecd861b73 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -20,12 +20,14 @@ // Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. // -#include #include #include +#include +#include #include -#include #include +#include +#include #include #include "gambit.h" @@ -373,32 +375,18 @@ 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) { - return false; - } - if (!m_parent) { - return true; + // TODO: Currently O(S) per call where S = number of subgames. + // Will become O(1) when GameSubgameRep adds a back-pointer (like m_infoset). + if (m_children.empty()) { + return !GetParent(); } - // 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; - } - } + auto *tree_game = static_cast(m_game); + if (tree_game->m_subgames.empty()) { + tree_game->BuildSubgameRoots(); } - return true; + return contains(tree_game->m_subgames, const_cast(this)); } bool GameNodeRep::IsStrategyReachable() const @@ -936,6 +924,7 @@ void GameTreeRep::ClearComputedValues() const m_ownPriorActionInfo = nullptr; const_cast(this)->m_unreachableNodes = nullptr; m_absentMindedInfosets.clear(); + m_subgames.clear(); m_computedValues = false; } @@ -1139,6 +1128,154 @@ void GameTreeRep::BuildUnreachableNodes() const } } +void GameTreeRep::BuildSubgameRoots() const +{ + if (!m_subgames.empty()) { + return; + } + + struct Range { + int min = std::numeric_limits::max(); + int max = 0; + + void Merge(const Range &p_source) + { + if (p_source.min < min) { + min = p_source.min; + } + if (p_source.max > max) { + max = p_source.max; + } + } + + bool operator==(const Range &p_other) const + { + return min == p_other.min && max == p_other.max; + } + }; + + std::unordered_map disc; + std::unordered_map hull; + int terminal_nodes_counter = 0; + + // Interval Assignment + struct IntervalVisitor { + std::unordered_map &m_disc; + int &m_counter; + + IntervalVisitor(std::unordered_map &p_disc, int &p_counter) + : m_disc(p_disc), m_counter(p_counter) + { + } + + static DFSCallbackResult OnEnter(GameNode, int) { return DFSCallbackResult::Continue; } + static DFSCallbackResult OnAction(GameNode, GameNode, int) + { + return DFSCallbackResult::Continue; + } + static void OnVisit(GameNode, int) {} + + DFSCallbackResult OnExit(const GameNode &p_node, int) + { + GameNodeRep *node = p_node.get(); + if (p_node->IsTerminal()) { + m_counter++; + m_disc[node] = {m_counter, m_counter}; + } + else { + Range &node_disc = m_disc[node]; + for (const auto &child : p_node->GetChildren()) { + node_disc.Merge(m_disc.at(child.get())); + } + } + return DFSCallbackResult::Continue; + } + }; + + // Adaptation of the Tarjan's bridge-finding algorithm: the game tree itself is the spanning tree + struct SubgameVisitor { + std::unordered_map &m_disc; + std::unordered_map &m_hull; + std::unordered_map m_low; + std::vector &m_subgames; + + SubgameVisitor(std::unordered_map &p_disc, + std::unordered_map &p_hull, + std::vector &p_subgames) + : m_disc(p_disc), m_hull(p_hull), m_subgames(p_subgames) + { + } + + static DFSCallbackResult OnEnter(GameNode, int) { return DFSCallbackResult::Continue; } + static DFSCallbackResult OnAction(GameNode, GameNode, int) + { + return DFSCallbackResult::Continue; + } + static void OnVisit(GameNode, int) {} + + DFSCallbackResult OnExit(const GameNode &p_node, int) + { + GameNodeRep *node = p_node.get(); + + const Range &own_disc = m_disc.at(node); + Range low = own_disc; + + // Expand via Information Set (Horizontal Edges) + if (auto *infoset = node->m_infoset) { + low.Merge(m_hull.at(infoset)); + } + + // Expand via Children (Vertical Edges) + for (const auto &child : p_node->GetChildren()) { + low.Merge(m_low.at(child.get())); + } + + m_low[node] = low; + + // Bridge Test + if (!p_node->IsTerminal() && low == own_disc) { + m_subgames.push_back(node); + } + + return DFSCallbackResult::Continue; + } + }; + + auto game = std::const_pointer_cast(shared_from_this()); + + // Step 1: Compute node intervals (analogous to Tarjan's disc) + IntervalVisitor interval_visitor(disc, terminal_nodes_counter); + WalkDFS(game, m_root, TraversalOrder::Postorder, interval_visitor); + + // Step 2: Compute infoset hulls + for (const auto &player : GetPlayersWithChance()) { + for (const auto &infoset : player->m_infosets) { + Range &infoset_hull = hull[infoset.get()]; + for (const auto &member : infoset->m_members) { + infoset_hull.Merge(disc.at(member.get())); + } + } + } + + // Step 3: Detect subgame roots (analogous to Tarjan's low) + SubgameVisitor subgame_visitor(disc, hull, const_cast &>(m_subgames)); + WalkDFS(game, m_root, TraversalOrder::Postorder, subgame_visitor); +} + +std::vector GameTreeRep::GetSubgames() const +{ + if (m_subgames.empty()) { + BuildSubgameRoots(); + } + + std::vector result; + result.reserve(m_subgames.size()); + for (auto *rep : m_subgames) { + result.emplace_back(rep->shared_from_this()); + } + return result; +} + //------------------------------------------------------------------------ // GameTreeRep: Writing data files //------------------------------------------------------------------------ diff --git a/src/games/gametree.h b/src/games/gametree.h index 03247b661..aaf86ff7f 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -47,6 +47,7 @@ class GameTreeRep final : public GameExplicitRep { mutable std::shared_ptr m_ownPriorActionInfo; mutable std::unique_ptr> m_unreachableNodes; mutable std::set m_absentMindedInfosets; + mutable std::vector m_subgames; /// @name Private auxiliary functions //@{ @@ -98,6 +99,7 @@ class GameTreeRep final : 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; + std::vector GetSubgames() const override; //@} /// @name Players @@ -182,6 +184,7 @@ class GameTreeRep final : public GameExplicitRep { std::vector BuildConsistentPlaysRecursiveImpl(GameNodeRep *node); void BuildOwnPriorActions() const; void BuildUnreachableNodes() const; + void BuildSubgameRoots() const; }; template class TreeMixedStrategyProfileRep : public MixedStrategyProfileRep { From ba9327e0489f8eba73aa96ac056baa3278066580 Mon Sep 17 00:00:00 2001 From: drdkad Date: Sun, 1 Feb 2026 10:53:41 +0000 Subject: [PATCH 03/10] Update ChangeLog --- ChangeLog | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index 09faf69fd..82951e1e5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -5,13 +5,15 @@ ### Changed - `Game.comment` has been renamed to `Game.description` +### Added +- Implement `GetSubgames()` (C++) to retrieve the list of subgame roots in standard post-order. + Detection uses a linear interval reachability algorithm adapted from the Tarjan's bridge-finder (IPL, 1974). ## [16.5.1] - unreleased ### Fixed - `Game.reveal` raised a null pointer access exception or dumped core in some cases (#749) - ## [16.5.0] - 2026-01-05 ### Fixed From 963115c5fa25a093ae0976ed52ebeb12f6fbf83f Mon Sep 17 00:00:00 2001 From: drdkad Date: Fri, 6 Mar 2026 09:21:30 +0000 Subject: [PATCH 04/10] Two-phase algorithm refactoring --- src/games/gametree.cc | 58 ++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index ecd861b73..70dff9dbc 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1158,13 +1158,15 @@ void GameTreeRep::BuildSubgameRoots() const std::unordered_map hull; int terminal_nodes_counter = 0; - // Interval Assignment - struct IntervalVisitor { + // Phase 1: Compute Subtree Spans and Information Set Hulls + struct SpanVisitor { std::unordered_map &m_disc; + std::unordered_map &m_hull; int &m_counter; - IntervalVisitor(std::unordered_map &p_disc, int &p_counter) - : m_disc(p_disc), m_counter(p_counter) + SpanVisitor(std::unordered_map &p_disc, + std::unordered_map &p_hull, int &p_counter) + : m_disc(p_disc), m_hull(p_hull), m_counter(p_counter) { } @@ -1187,21 +1189,22 @@ void GameTreeRep::BuildSubgameRoots() const for (const auto &child : p_node->GetChildren()) { node_disc.Merge(m_disc.at(child.get())); } + m_hull[node->m_infoset].Merge(node_disc); } return DFSCallbackResult::Continue; } }; - // Adaptation of the Tarjan's bridge-finding algorithm: the game tree itself is the spanning tree - struct SubgameVisitor { + // Phase 2: Reachability and Detection + struct BridgeVisitor { std::unordered_map &m_disc; std::unordered_map &m_hull; std::unordered_map m_low; std::vector &m_subgames; - SubgameVisitor(std::unordered_map &p_disc, - std::unordered_map &p_hull, - std::vector &p_subgames) + BridgeVisitor(std::unordered_map &p_disc, + std::unordered_map &p_hull, + std::vector &p_subgames) : m_disc(p_disc), m_hull(p_hull), m_subgames(p_subgames) { } @@ -1217,23 +1220,20 @@ void GameTreeRep::BuildSubgameRoots() const { GameNodeRep *node = p_node.get(); - const Range &own_disc = m_disc.at(node); - Range low = own_disc; - - // Expand via Information Set (Horizontal Edges) - if (auto *infoset = node->m_infoset) { - low.Merge(m_hull.at(infoset)); + if (p_node->IsTerminal()) { + m_low[node] = m_disc.at(node); + return DFSCallbackResult::Continue; } - // Expand via Children (Vertical Edges) + Range low = m_hull.at(node->m_infoset); + for (const auto &child : p_node->GetChildren()) { low.Merge(m_low.at(child.get())); } m_low[node] = low; - // Bridge Test - if (!p_node->IsTerminal() && low == own_disc) { + if (low == m_disc.at(node)) { m_subgames.push_back(node); } @@ -1243,23 +1243,13 @@ void GameTreeRep::BuildSubgameRoots() const auto game = std::const_pointer_cast(shared_from_this()); - // Step 1: Compute node intervals (analogous to Tarjan's disc) - IntervalVisitor interval_visitor(disc, terminal_nodes_counter); - WalkDFS(game, m_root, TraversalOrder::Postorder, interval_visitor); - - // Step 2: Compute infoset hulls - for (const auto &player : GetPlayersWithChance()) { - for (const auto &infoset : player->m_infosets) { - Range &infoset_hull = hull[infoset.get()]; - for (const auto &member : infoset->m_members) { - infoset_hull.Merge(disc.at(member.get())); - } - } - } + // Phase 1: Compute subtree spans D(v) and information set hulls H(I) + SpanVisitor span_visitor(disc, hull, terminal_nodes_counter); + WalkDFS(game, m_root, TraversalOrder::Postorder, span_visitor); - // Step 3: Detect subgame roots (analogous to Tarjan's low) - SubgameVisitor subgame_visitor(disc, hull, const_cast &>(m_subgames)); - WalkDFS(game, m_root, TraversalOrder::Postorder, subgame_visitor); + // Phase 2: Compute reachable spans L(v) and detect subgame roots + BridgeVisitor bridge_visitor(disc, hull, const_cast &>(m_subgames)); + WalkDFS(game, m_root, TraversalOrder::Postorder, bridge_visitor); } std::vector GameTreeRep::GetSubgames() const From 0d6b9d31dace7f28a4ceb3d99ccb0a3b9b6b8ff0 Mon Sep 17 00:00:00 2001 From: drdkad Date: Mon, 9 Mar 2026 06:49:30 +0000 Subject: [PATCH 05/10] Tighten BuildSubgameRoots: eliminate redundant constructors and copies --- src/games/gametree.cc | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 70dff9dbc..733f75076 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1156,19 +1156,12 @@ void GameTreeRep::BuildSubgameRoots() const std::unordered_map disc; std::unordered_map hull; - int terminal_nodes_counter = 0; - // Phase 1: Compute Subtree Spans and Information Set Hulls + // Phase 1: Compute subtree spans and infoset hulls struct SpanVisitor { std::unordered_map &m_disc; std::unordered_map &m_hull; - int &m_counter; - - SpanVisitor(std::unordered_map &p_disc, - std::unordered_map &p_hull, int &p_counter) - : m_disc(p_disc), m_hull(p_hull), m_counter(p_counter) - { - } + int m_counter = 0; static DFSCallbackResult OnEnter(GameNode, int) { return DFSCallbackResult::Continue; } static DFSCallbackResult OnAction(GameNode, GameNode, int) @@ -1195,19 +1188,12 @@ void GameTreeRep::BuildSubgameRoots() const } }; - // Phase 2: Reachability and Detection + // Phase 2: Reachability and detection struct BridgeVisitor { - std::unordered_map &m_disc; - std::unordered_map &m_hull; - std::unordered_map m_low; + const std::unordered_map &m_disc; + const std::unordered_map &m_hull; std::vector &m_subgames; - - BridgeVisitor(std::unordered_map &p_disc, - std::unordered_map &p_hull, - std::vector &p_subgames) - : m_disc(p_disc), m_hull(p_hull), m_subgames(p_subgames) - { - } + std::unordered_map m_low; static DFSCallbackResult OnEnter(GameNode, int) { return DFSCallbackResult::Continue; } static DFSCallbackResult OnAction(GameNode, GameNode, int) @@ -1225,14 +1211,13 @@ void GameTreeRep::BuildSubgameRoots() const return DFSCallbackResult::Continue; } - Range low = m_hull.at(node->m_infoset); + Range &low = m_low[node]; + low = m_hull.at(node->m_infoset); for (const auto &child : p_node->GetChildren()) { low.Merge(m_low.at(child.get())); } - m_low[node] = low; - if (low == m_disc.at(node)) { m_subgames.push_back(node); } @@ -1243,12 +1228,10 @@ void GameTreeRep::BuildSubgameRoots() const auto game = std::const_pointer_cast(shared_from_this()); - // Phase 1: Compute subtree spans D(v) and information set hulls H(I) - SpanVisitor span_visitor(disc, hull, terminal_nodes_counter); + SpanVisitor span_visitor{disc, hull}; WalkDFS(game, m_root, TraversalOrder::Postorder, span_visitor); - // Phase 2: Compute reachable spans L(v) and detect subgame roots - BridgeVisitor bridge_visitor(disc, hull, const_cast &>(m_subgames)); + BridgeVisitor bridge_visitor{disc, hull, m_subgames}; WalkDFS(game, m_root, TraversalOrder::Postorder, bridge_visitor); } From 5edb2089e2403c03f6f88fc4992be0d85ac4cbd4 Mon Sep 17 00:00:00 2001 From: drdkad Date: Mon, 9 Mar 2026 08:19:34 +0000 Subject: [PATCH 06/10] Compute span via front/back children, eliminate redundant terminal initialization in Phase 2 --- src/games/gametree.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 733f75076..cb7c7222f 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1179,9 +1179,9 @@ void GameTreeRep::BuildSubgameRoots() const } else { Range &node_disc = m_disc[node]; - for (const auto &child : p_node->GetChildren()) { - node_disc.Merge(m_disc.at(child.get())); - } + const auto &children = p_node->GetChildren(); + node_disc.min = m_disc.at(children.front().get()).min; + node_disc.max = m_disc.at(children.back().get()).max; m_hull[node->m_infoset].Merge(node_disc); } return DFSCallbackResult::Continue; @@ -1204,13 +1204,11 @@ void GameTreeRep::BuildSubgameRoots() const DFSCallbackResult OnExit(const GameNode &p_node, int) { - GameNodeRep *node = p_node.get(); - if (p_node->IsTerminal()) { - m_low[node] = m_disc.at(node); return DFSCallbackResult::Continue; } + GameNodeRep *node = p_node.get(); Range &low = m_low[node]; low = m_hull.at(node->m_infoset); @@ -1231,7 +1229,7 @@ void GameTreeRep::BuildSubgameRoots() const SpanVisitor span_visitor{disc, hull}; WalkDFS(game, m_root, TraversalOrder::Postorder, span_visitor); - BridgeVisitor bridge_visitor{disc, hull, m_subgames}; + BridgeVisitor bridge_visitor{disc, hull, m_subgames, disc}; WalkDFS(game, m_root, TraversalOrder::Postorder, bridge_visitor); } From d951c792b14cdbc4983d38eaeb4b85ffccb66105 Mon Sep 17 00:00:00 2001 From: drdkad Date: Tue, 10 Mar 2026 08:57:31 +0000 Subject: [PATCH 07/10] Initialize L-values inline in Phase 2 instead of copying disc map --- src/games/gametree.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index cb7c7222f..79556fce6 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1204,11 +1204,12 @@ void GameTreeRep::BuildSubgameRoots() const DFSCallbackResult OnExit(const GameNode &p_node, int) { + GameNodeRep *node = p_node.get(); if (p_node->IsTerminal()) { + m_low[node] = m_disc.at(node); return DFSCallbackResult::Continue; } - GameNodeRep *node = p_node.get(); Range &low = m_low[node]; low = m_hull.at(node->m_infoset); @@ -1229,7 +1230,7 @@ void GameTreeRep::BuildSubgameRoots() const SpanVisitor span_visitor{disc, hull}; WalkDFS(game, m_root, TraversalOrder::Postorder, span_visitor); - BridgeVisitor bridge_visitor{disc, hull, m_subgames, disc}; + BridgeVisitor bridge_visitor{disc, hull, m_subgames}; WalkDFS(game, m_root, TraversalOrder::Postorder, bridge_visitor); } From 34507aeb51937eca45bdebadd0c9f9b1d1b1e6c7 Mon Sep 17 00:00:00 2001 From: Ted Turocy Date: Thu, 12 Mar 2026 09:50:37 +0000 Subject: [PATCH 08/10] Update gametree.cc --- src/games/gametree.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 79556fce6..2f3b346c3 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1140,12 +1140,8 @@ void GameTreeRep::BuildSubgameRoots() const void Merge(const Range &p_source) { - if (p_source.min < min) { - min = p_source.min; - } - if (p_source.max > max) { - max = p_source.max; - } + min = std::min(min, p_source.min); + max = std::max(max, p_source.max); } bool operator==(const Range &p_other) const From a69ca6e0acecdd6d34f798898d187b623bcc25f1 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Thu, 12 Mar 2026 09:56:38 +0000 Subject: [PATCH 09/10] Tidy use of min/max in range --- src/games/gametree.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 2f3b346c3..c66a400d8 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -1135,18 +1135,18 @@ void GameTreeRep::BuildSubgameRoots() const } struct Range { - int min = std::numeric_limits::max(); - int max = 0; + int m_min = std::numeric_limits::max(); + int m_max = 0; void Merge(const Range &p_source) { - min = std::min(min, p_source.min); - max = std::max(max, p_source.max); + m_min = std::min(m_min, p_source.m_min); + m_max = std::max(m_max, p_source.m_max); } bool operator==(const Range &p_other) const { - return min == p_other.min && max == p_other.max; + return m_min == p_other.m_min && m_max == p_other.m_max; } }; @@ -1176,8 +1176,8 @@ void GameTreeRep::BuildSubgameRoots() const else { Range &node_disc = m_disc[node]; const auto &children = p_node->GetChildren(); - node_disc.min = m_disc.at(children.front().get()).min; - node_disc.max = m_disc.at(children.back().get()).max; + node_disc.m_min = m_disc.at(children.front().get()).m_min; + node_disc.m_max = m_disc.at(children.back().get()).m_max; m_hull[node->m_infoset].Merge(node_disc); } return DFSCallbackResult::Continue; From 31123d828d01d14ac0126d077c59bd6f334d2aa4 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Thu, 12 Mar 2026 10:05:46 +0000 Subject: [PATCH 10/10] Revised ChangeLog for clarity --- ChangeLog | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0222da18d..e20c9a7fa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,8 +6,9 @@ - `Game.comment` has been renamed to `Game.description` ### Added -- Implement `GetSubgames()` (C++) to retrieve the list of subgame roots in standard post-order. - Detection uses a linear interval reachability algorithm adapted from the Tarjan's bridge-finder (IPL, 1974). +- Implement linear-time algorithm to find all root nodes of proper subgames, using an adaptation of + Tarjan's (1974) algorithm for finding bridges in an undirected graph. Subgame roots are cached so + subsequent lookup is constant-time (if the game is unchanged). (#584) ### Fixed - `enumpoly` would take a very long time on some supports where an equilibrium is located on the @@ -15,6 +16,8 @@ these out; these will always be found by another projection. (#756) - In the graphical interface, the logit correspondence display would fail and terminate the program on very small (<10^{-300}) probabilities. +- The new subgame root computation fixes a bug which failed to detect subgames where the subgame + root node is a member of an absent-minded infoset. (#584) ## [16.5.1] - unreleased