From f95eceebdab7d636afec2cdd75200ca7677391e8 Mon Sep 17 00:00:00 2001 From: drdkad Date: Fri, 2 May 2025 09:57:41 +0100 Subject: [PATCH 1/2] Fix bug in m_numNodes counter when doing operations involving deletion --- src/games/gametree.cc | 4 +++- tests/test_node.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 8b358b7c0..20f177010 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -138,6 +138,7 @@ void GameTreeRep::DeleteAction(GameAction p_action) for (auto member : infoset->m_members) { DeleteTree(member->m_children[where]); + m_numNodes--; member->m_children[where]->Invalidate(); erase_atindex(member->m_children, where); } @@ -434,6 +435,7 @@ void GameTreeRep::DeleteParent(GameNode p_node) std::find(oldParent->m_children.begin(), oldParent->m_children.end(), node)); DeleteTree(oldParent); node->m_parent = oldParent->m_parent; + m_numNodes--; if (node->m_parent) { std::replace(node->m_parent->m_children.begin(), node->m_parent->m_children.end(), oldParent, node); @@ -456,6 +458,7 @@ void GameTreeRep::DeleteTree(GameNode p_node) IncrementVersion(); while (!node->m_children.empty()) { DeleteTree(node->m_children.front()); + m_numNodes--; node->m_children.front()->Invalidate(); erase_atindex(node->m_children, 1); } @@ -463,7 +466,6 @@ void GameTreeRep::DeleteTree(GameNode p_node) RemoveMember(node->m_infoset, node); node->m_infoset = nullptr; } - m_numNodes--; node->m_outcome = nullptr; node->m_label = ""; diff --git a/tests/test_node.py b/tests/test_node.py index 6d26e9d07..8c39ac7c2 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -446,7 +446,7 @@ def test_len_after_delete_tree(): list_nodes = list(game.nodes) root_of_the_deleted_subtree = list_nodes[3] - number_of_deleted_nodes = _count_subtree_nodes(root_of_the_deleted_subtree) + number_of_deleted_nodes = _count_subtree_nodes(root_of_the_deleted_subtree) - 1 game.delete_tree(root_of_the_deleted_subtree) From 767f11fc50738b5712e43de4e8d3b1f24729a037 Mon Sep 17 00:00:00 2001 From: drdkad Date: Fri, 2 May 2025 10:13:02 +0100 Subject: [PATCH 2/2] add m_numNonterminalNodes field to GameRep --- src/games/game.h | 2 + src/games/gameagg.h | 2 + src/games/gamebagg.h | 2 + src/games/gametable.h | 2 + src/games/gametree.cc | 6 ++ src/games/gametree.h | 3 + src/pygambit/gambit.pxd | 1 + src/pygambit/game.pxi | 44 +++++++++ tests/test_node.py | 213 +++++++++++++++++++++++++++++++++++++--- 9 files changed, 261 insertions(+), 14 deletions(-) diff --git a/src/games/game.h b/src/games/game.h index 19d218957..12aef1f85 100644 --- a/src/games/game.h +++ b/src/games/game.h @@ -634,6 +634,8 @@ class GameRep : public BaseGameRep { virtual GameNode GetRoot() const = 0; /// Returns the number of nodes in the game virtual size_t NumNodes() const = 0; + /// Returns the number of non-terminal nodes in the game + virtual size_t NumNonterminalNodes() const = 0; //@} /// @name Modification diff --git a/src/games/gameagg.h b/src/games/gameagg.h index 64c59c079..7390714a8 100644 --- a/src/games/gameagg.h +++ b/src/games/gameagg.h @@ -101,6 +101,8 @@ class GameAGGRep : public GameRep { GameNode GetRoot() const override { throw UndefinedException(); } /// Returns the number of nodes in the game size_t NumNodes() const override { throw UndefinedException(); } + /// Returns the number of non-terminal nodes in the game + size_t NumNonterminalNodes() const override { throw UndefinedException(); } //@} /// @name General data access diff --git a/src/games/gamebagg.h b/src/games/gamebagg.h index f8867eef3..0a576f961 100644 --- a/src/games/gamebagg.h +++ b/src/games/gamebagg.h @@ -109,6 +109,8 @@ class GameBAGGRep : public GameRep { GameNode GetRoot() const override { throw UndefinedException(); } /// Returns the number of nodes in the game size_t NumNodes() const override { throw UndefinedException(); } + /// Returns the number of non-terminal nodes in the game + size_t NumNonterminalNodes() const override { throw UndefinedException(); } //@} /// @name General data access diff --git a/src/games/gametable.h b/src/games/gametable.h index 640353573..a676bf44e 100644 --- a/src/games/gametable.h +++ b/src/games/gametable.h @@ -90,6 +90,8 @@ class GameTableRep : public GameExplicitRep { GameNode GetRoot() const override { throw UndefinedException(); } /// Returns the number of nodes in the game size_t NumNodes() const override { throw UndefinedException(); } + /// Returns the number of non-terminal nodes in the game + size_t NumNonterminalNodes() const override { throw UndefinedException(); } //@} /// @name Outcomes diff --git a/src/games/gametree.cc b/src/games/gametree.cc index 20f177010..839e668d5 100644 --- a/src/games/gametree.cc +++ b/src/games/gametree.cc @@ -242,6 +242,7 @@ GameAction GameTreeRep::InsertAction(GameInfoset p_infoset, GameAction p_action } m_numNodes += p_infoset->m_members.size(); + // m_numNonterminalNodes stays unchanged when an action is appended to an information set ClearComputedValues(); Canonicalize(); return action; @@ -455,6 +456,9 @@ void GameTreeRep::DeleteTree(GameNode p_node) throw MismatchException(); } GameNodeRep *node = p_node; + if (!node->IsTerminal()) { + m_numNonterminalNodes--; + } IncrementVersion(); while (!node->m_children.empty()) { DeleteTree(node->m_children.front()); @@ -633,6 +637,7 @@ GameInfoset GameTreeRep::AppendMove(GameNode p_node, GameInfoset p_infoset) node->m_children.push_back(new GameNodeRep(this, node)); m_numNodes++; }); + m_numNonterminalNodes++; ClearComputedValues(); Canonicalize(); return node->m_infoset; @@ -681,6 +686,7 @@ GameInfoset GameTreeRep::InsertMove(GameNode p_node, GameInfoset p_infoset) // Total nodes added = 1 (newNode) + (NumActions - 1) (new children of newNode) = NumActions m_numNodes += newNode->m_infoset->m_actions.size(); + m_numNonterminalNodes++; ClearComputedValues(); Canonicalize(); return p_infoset; diff --git a/src/games/gametree.h b/src/games/gametree.h index d63d63349..76347dba2 100644 --- a/src/games/gametree.h +++ b/src/games/gametree.h @@ -37,6 +37,7 @@ class GameTreeRep : public GameExplicitRep { GameNodeRep *m_root; GamePlayerRep *m_chance; std::size_t m_numNodes = 1; + std::size_t m_numNonterminalNodes = 0; /// @name Private auxiliary functions //@{ @@ -95,6 +96,8 @@ class GameTreeRep : public GameExplicitRep { GameNode GetRoot() const override { return m_root; } /// Returns the number of nodes in the game size_t NumNodes() const override { return m_numNodes; } + /// Returns the number of non-terminal nodes in the game + size_t NumNonterminalNodes() const override { return m_numNonterminalNodes; } //@} void DeleteOutcome(const GameOutcome &) override; diff --git a/src/pygambit/gambit.pxd b/src/pygambit/gambit.pxd index f08d077da..ec9eff88a 100644 --- a/src/pygambit/gambit.pxd +++ b/src/pygambit/gambit.pxd @@ -187,6 +187,7 @@ cdef extern from "games/game.h": void DeleteOutcome(c_GameOutcome) except + int NumNodes() except + + int NumNonterminalNodes() except + c_GameNode GetRoot() except + c_GameStrategy GetStrategy(int) except +IndexError diff --git a/src/pygambit/game.pxi b/src/pygambit/game.pxi index 2c1aacac6..83142ad6d 100644 --- a/src/pygambit/game.pxi +++ b/src/pygambit/game.pxi @@ -194,6 +194,41 @@ class GameNodes: yield from dfs(Node.wrap(self.game.deref().GetRoot())) +@cython.cclass +class GameNonterminalNodes: + """Represents the set of nodes in a game.""" + game = cython.declare(c_Game) + + def __init__(self, *args, **kwargs) -> None: + raise ValueError("Cannot create GameNonterminalNodes outside a Game.") + + @staticmethod + @cython.cfunc + def wrap(game: c_Game) -> GameNonterminalNodes: + obj: GameNonterminalNodes = GameNonterminalNodes.__new__(GameNonterminalNodes) + obj.game = game + return obj + + def __repr__(self) -> str: + return f"GameNonterminalNodes(game={Game.wrap(self.game)})" + + def __len__(self) -> int: + """The number of non-terminal nodes in the game.""" + if not self.game.deref().IsTree(): + return 0 + return self.game.deref().NumNonterminalNodes() + + def __iter__(self) -> typing.Iterator[Node]: + def dfs(node): + if not node.is_terminal: + yield node + for child in node.children: + yield from dfs(child) + if not self.game.deref().IsTree(): + return + yield from dfs(Node.wrap(self.game.deref().GetRoot())) + + @cython.cclass class GameOutcomes: """Represents the set of outcomes in a game.""" @@ -713,6 +748,15 @@ class Game: """ return GameNodes.wrap(self.game) + @property + def _nonterminal_nodes(self) -> GameNonterminalNodes: + """The set of non-terminal nodes in the game. + + Iteration over this property yields the non-terminal nodes in the order of depth-first + search. + """ + return GameNonterminalNodes.wrap(self.game) + @property def contingencies(self) -> pygambit.gameiter.Contingencies: """An iterator over the contingencies in the game.""" diff --git a/tests/test_node.py b/tests/test_node.py index 8c39ac7c2..b4c25595e 100644 --- a/tests/test_node.py +++ b/tests/test_node.py @@ -418,16 +418,25 @@ def _get_members(action: gbt.Action) -> set[gbt.Node]: return [member_node.children[action_index] for member_node in infoset.members] -def _count_subtree_nodes(start_node: gbt.Node) -> int: - """Counts nodes in the subtree rooted at start_node (including start_node).""" - count = 1 +def _count_subtree_nodes(start_node: gbt.Node, count_terminal: bool) -> int: + """Counts nodes in the subtree rooted at `start_node` (including `start_node`). + + Parameters + ---------- + start_node: Node + The root of the subtree + count_terminal: bool + Include or exclude terminal nodes from count + """ + count = 1 if count_terminal or not start_node.is_terminal else 0 + for child in start_node.children: - count += _count_subtree_nodes(child) + count += _count_subtree_nodes(child, count_terminal) return count -def test_len_matches_sum_children_plus_one(): - """Verify `len(game.nodes)` matches (sum of children counts + 1) +def test_len_matches_expected_node_count(): + """Verify `len(game.nodes)` matches expected node count """ game = games.read_from_file("e01.efg") expected_node_count = 9 @@ -435,7 +444,7 @@ def test_len_matches_sum_children_plus_one(): direct_len = len(game.nodes) assert direct_len == expected_node_count - assert direct_len == _count_subtree_nodes(game.root) + assert direct_len == _count_subtree_nodes(game.root, True) def test_len_after_delete_tree(): @@ -446,7 +455,7 @@ def test_len_after_delete_tree(): list_nodes = list(game.nodes) root_of_the_deleted_subtree = list_nodes[3] - number_of_deleted_nodes = _count_subtree_nodes(root_of_the_deleted_subtree) - 1 + number_of_deleted_nodes = _count_subtree_nodes(root_of_the_deleted_subtree, True) - 1 game.delete_tree(root_of_the_deleted_subtree) @@ -462,8 +471,8 @@ def test_len_after_delete_parent(): node_parent_to_delete = list_nodes[4] - number_of_node_ancestors = _count_subtree_nodes(node_parent_to_delete) - number_of_parent_ancestors = _count_subtree_nodes(node_parent_to_delete.parent) + number_of_node_ancestors = _count_subtree_nodes(node_parent_to_delete, True) + number_of_parent_ancestors = _count_subtree_nodes(node_parent_to_delete.parent, True) diff = number_of_parent_ancestors - number_of_node_ancestors game.delete_parent(node_parent_to_delete) @@ -533,7 +542,7 @@ def test_len_after_delete_action(): action_nodes = _get_members(action_to_delete) for subtree_root in action_nodes: - nodes_to_delete += _count_subtree_nodes(subtree_root) + nodes_to_delete += _count_subtree_nodes(subtree_root, True) game.delete_action(action_to_delete) @@ -580,10 +589,186 @@ def test_len_after_copy_tree(): game = games.read_from_file("e01.efg") initial_number_of_nodes = len(game.nodes) list_nodes = list(game.nodes) - src_node = list_nodes[3] # path=[1, 0] - dest_node = list_nodes[2] # path=[0, 0] - number_of_src_ancestors = _count_subtree_nodes(src_node) + src_node = list_nodes[3] # path=[1, 0] + dest_node = list_nodes[2] # path=[0, 0] + number_of_src_ancestors = _count_subtree_nodes(src_node, True) game.copy_tree(src_node, dest_node) assert len(game.nodes) == initial_number_of_nodes + number_of_src_ancestors - 1 + + +def test_nonterminal_len_matches_expected_count(): + """Verify `len(game._nonterminal_nodes)` matches expected count + """ + game = games.read_from_file("e01.efg") + expected_nonterminal_node_count = 4 + + direct_nonterminal_len = len(game._nonterminal_nodes) + assert direct_nonterminal_len == expected_nonterminal_node_count + + +def test_nonterminal_len_after_delete_tree(): + """Verify `len(game._nonterminal_nodes)` is correct after `delete_tree`. + """ + game = games.read_from_file("e01.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + list_nodes = list(game.nodes) + + root_of_the_deleted_subtree = list_nodes[1] + number_of_deleted_nonterminal_nodes = _count_subtree_nodes(root_of_the_deleted_subtree, False) + + game.delete_tree(root_of_the_deleted_subtree) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes \ + - number_of_deleted_nonterminal_nodes + + +def test_nonterminal_len_after_delete_parent_of_nonterminal_node(): + """Verify `len(game._nonterminal_nodes)` is correct after `delete_parent`. + """ + game = games.read_from_file("e02.efg") + list_nodes = list(game.nodes) + node_parent_to_delete = list_nodes[4] # path=[1, 1] + + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + diff = _count_subtree_nodes(node_parent_to_delete.parent, False) \ + - _count_subtree_nodes(node_parent_to_delete, False) + + game.delete_parent(node_parent_to_delete) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes - diff + + +def test_nonterminal_len_after_delete_parent_of_terminal_node(): + """Verify `len(game._nonterminal_nodes)` is correct after `delete_parent`. + """ + game = games.read_from_file("e02.efg") + list_nodes = list(game.nodes) + node_parent_to_delete = list_nodes[5] # path=[0, 1, 1] + + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + diff = _count_subtree_nodes(node_parent_to_delete.parent, False) \ + - _count_subtree_nodes(node_parent_to_delete, False) + + game.delete_parent(node_parent_to_delete) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes - diff + + +def test_nonterminal_len_after_append_move(): + """Verify `len(game._nonterminal_nodes)` is correct after `append_move`. + """ + game = games.read_from_file("e01.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + list_nodes = list(game.nodes) + + terminal_node = list_nodes[5] # path=[1, 1, 0] + player = game.players[0] + actions_to_add = ["T", "M", "B"] + + game.append_move(terminal_node, player, actions_to_add) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes \ + + _count_subtree_nodes(terminal_node, False) + + +def test_nonterminal_len_after_append_infoset(): + """Verify `len(game._nonterminal_nodes)` is correct after `append_infoset`. + """ + game = games.read_from_file("e02.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + list_nodes = list(game.nodes) + + member_node = list_nodes[2] # path=[1] + infoset_to_modify = member_node.infoset + terminal_node_to_add = list_nodes[6] # path=[1, 1, 1] + + game.append_infoset(terminal_node_to_add, infoset_to_modify) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes \ + + _count_subtree_nodes(terminal_node_to_add, False) + + +def test_nonterminal_len_after_add_action(): + """Verify `len(game._nonterminal_nodes)` does not change after `add_action` to an infoset. + """ + game = games.read_from_file("e01.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + + infoset_to_modify = game.infosets[1] + + game.add_action(infoset_to_modify) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes + + +def test_nonterminal_len_after_delete_action(): + """Verify `len(game._nonterminal_nodes)` is correct after `delete_action`. + """ + game = games.read_from_file("e02.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + + action_to_delete = game.infosets[0].actions[1] + + # Calculate the total number of nodes within all subtrees + # that begin immediately after taking the specified action. + nonterminal_nodes_to_delete = 0 + action_nodes = _get_members(action_to_delete) + + for subtree_root in action_nodes: + nonterminal_nodes_to_delete += _count_subtree_nodes(subtree_root, False) + + game.delete_action(action_to_delete) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes \ + - nonterminal_nodes_to_delete + + +def test_nonterminal_len_after_insert_move(): + """Verify `len(game._nonterminal_nodes)` correctly increaces by 1 after `insert_move`. + """ + game = games.read_from_file("e01.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + list_nodes = list(game.nodes) + + node_to_insert_above = list_nodes[3] + + player = game.players[1] + num_actions_to_add = 3 + + game.insert_move(node_to_insert_above, player, num_actions_to_add) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes + 1 + + +def test_nonterminal_len_after_insert_infoset(): + """Verify `len(game._nonterminal_nodes)` correctly increaces by 1 after `insert_infoset`. + """ + game = games.read_from_file("e01.efg") + initial_number_of_nonterminal_nodes = len(game._nonterminal_nodes) + list_nodes = list(game.nodes) + + member_node = list_nodes[6] # path=[1] + infoset_to_modify = member_node.infoset + node_to_insert_above = list_nodes[7] # path=[0, 1] + + game.insert_infoset(node_to_insert_above, infoset_to_modify) + + assert len(game._nonterminal_nodes) == initial_number_of_nonterminal_nodes + 1 + + +def test_nonterminal_len_after_copy_tree(): + """Verify `len(game._nonterminal_nodes)` is correct after `copy_tree`. + """ + game = games.read_from_file("e01.efg") + initial_number_of_nodes = len(game._nonterminal_nodes) + list_nodes = list(game.nodes) + src_node = list_nodes[3] # path=[1, 0] + dest_node = list_nodes[2] # path=[0, 0] + number_of_nonterminal_src_ancestors = _count_subtree_nodes(src_node, False) + + game.copy_tree(src_node, dest_node) + + assert len(game._nonterminal_nodes) == initial_number_of_nodes \ + + number_of_nonterminal_src_ancestors