Remove GameNodeRep::GetChild(int) and implement children as a std::vector

Closes #514

Author: tturocy

src/games/behavmixed.cc

@@ -115,7 +115,7 @@ void MixedBehaviorProfile<T>::RealizationProbs(const MixedStrategyProfile<T> &mp
       prob = T(node->m_infoset->GetActionProb(node->m_infoset->GetAction(i)));
     }
 
-    GameNodeRep *child = node->m_children[i];
+    GameNodeRep *child = node->m_children[i - 1];
 
     map_bvals[child] = prob * map_bvals[node];
     map_nvals[child] += map_bvals[child];
@@ -428,8 +428,8 @@ T MixedBehaviorProfile<T>::DiffActionValue(const GameAction &p_action,
 
     deriv += DiffRealizProb(member, p_oppAction) *
              (map_nodeValues[child][player] - map_actionValues[p_action]);
-    deriv += map_realizProbs[member] *
-             DiffNodeValue(member->GetChild(p_action->GetNumber()), player, p_oppAction);
+    deriv +=
+        map_realizProbs[member] * DiffNodeValue(member->GetChild(p_action), player, p_oppAction);
   }
 
   return deriv / GetInfosetProb(p_action->GetInfoset());

src/games/game.cc

@@ -124,7 +124,7 @@ void GamePlayerRep::MakeReducedStrats(GameNodeRep *n, GameNodeRep *nn)
         // we haven't visited this infoset before
         n->m_infoset->flag = 1;
         for (size_t i = 1; i <= n->NumChildren(); i++) {
-          GameNodeRep *m = n->m_children[i];
+          GameNodeRep *m = n->m_children[i - 1];
           n->whichbranch = m;
           n->m_infoset->whichbranch = i;
           MakeReducedStrats(m, nn);
@@ -133,19 +133,19 @@ void GamePlayerRep::MakeReducedStrats(GameNodeRep *n, GameNodeRep *nn)
       }
       else {
         // we have visited this infoset, take same action
-        MakeReducedStrats(n->m_children[n->m_infoset->whichbranch], nn);
+        MakeReducedStrats(n->m_children[n->m_infoset->whichbranch - 1], nn);
       }
     }
     else {
       n->ptr = nullptr;
       if (nn != nullptr) {
         n->ptr = nn->m_parent;
       }
-      n->whichbranch = n->m_children[1];
+      n->whichbranch = n->m_children.front();
       if (n->m_infoset) {
         n->m_infoset->whichbranch = 0;
       }
-      MakeReducedStrats(n->m_children[1], n->m_children[1]);
+      MakeReducedStrats(n->m_children.front(), n->m_children.front());
     }
   }
   else if (nn) {

src/games/game.h

@@ -367,7 +367,7 @@ class GameNodeRep : public GameObject {
   GameInfosetRep *m_infoset{nullptr};
   GameNodeRep *m_parent;
   GameOutcomeRep *m_outcome{nullptr};
-  Array<GameNodeRep *> m_children;
+  std::vector<GameNodeRep *> m_children;
   GameNodeRep *whichbranch{nullptr}, *ptr{nullptr};
 
   GameNodeRep(GameRep *e, GameNodeRep *p);
@@ -383,13 +383,12 @@ class GameNodeRep : public GameObject {
 
   int GetNumber() const { return m_number; }
   size_t NumChildren() const { return m_children.size(); }
-  GameNode GetChild(int i) const { return m_children[i]; }
   GameNode GetChild(const GameAction &p_action)
   {
     if (p_action->GetInfoset() != m_infoset) {
       throw MismatchException();
     }
-    return m_children[p_action->GetNumber()];
+    return m_children.at(p_action->GetNumber() - 1);
   }
   Array<GameNode> GetChildren() const;
 

src/games/gametree.cc

@@ -137,10 +137,10 @@ void GameTreeRep::DeleteAction(GameAction p_action)
   infoset->RenumberActions();
 
   for (auto member : infoset->m_members) {
-    DeleteTree(member->m_children[where]);
+    DeleteTree(member->m_children[where - 1]);
     m_numNodes--;
-    member->m_children[where]->Invalidate();
-    erase_atindex(member->m_children, where);
+    member->m_children[where - 1]->Invalidate();
+    member->m_children.erase(std::next(member->m_children.begin(), where - 1));
   }
   ClearComputedValues();
   Canonicalize();
@@ -461,10 +461,10 @@ void GameTreeRep::DeleteTree(GameNode p_node)
   }
   IncrementVersion();
   while (!node->m_children.empty()) {
-    DeleteTree(node->m_children.front());
+    DeleteTree(node->m_children.back());
     m_numNodes--;
-    node->m_children.front()->Invalidate();
-    erase_atindex(node->m_children, 1);
+    node->m_children.back()->Invalidate();
+    node->m_children.pop_back();
   }
   if (node->m_infoset) {
     RemoveMember(node->m_infoset, node);
@@ -738,9 +738,10 @@ Rational SubtreeSum(const GameNode &p_node)
   Rational sum(0);
 
   if (p_node->NumChildren() > 0) {
-    sum = SubtreeSum(p_node->GetChild(1));
-    for (size_t i = 2; i <= p_node->NumChildren(); i++) {
-      if (SubtreeSum(p_node->GetChild(i)) != sum) {
+    auto children = p_node->GetChildren();
+    sum = SubtreeSum(children.front());
+    for (auto child = std::next(children.begin()); child != children.end(); child++) {
+      if (SubtreeSum(*child) != sum) {
         throw NotZeroSumException();
       }
     }
@@ -776,17 +777,17 @@ bool GameTreeRep::IsPerfectRecall(GameInfoset &s1, GameInfoset &s2) const
         auto *iset2 = player->m_infosets[j - 1];
 
         bool precedes = false;
-        size_t action = 0;
+        GameAction action = nullptr;
 
         for (size_t m = 1; m <= iset2->NumMembers(); m++) {
           size_t n;
           for (n = 1; n <= iset1->NumMembers(); n++) {
             if (iset2->GetMember(m)->IsSuccessorOf(iset1->GetMember(n)) &&
                 iset1->GetMember(n) != iset2->GetMember(m)) {
               precedes = true;
-              for (size_t act = 1; act <= iset1->NumActions(); act++) {
+              for (const auto &act : iset1->GetActions()) {
                 if (iset2->GetMember(m)->IsSuccessorOf(iset1->GetMember(n)->GetChild(act))) {
-                  if (action != 0 && action != act) {
+                  if (action != nullptr && action != act) {
                     s1 = iset1;
                     s2 = iset2;
                     return false;

src/gui/efglayout.cc

@@ -574,16 +574,15 @@ int gbtTreeLayout::LayoutSubtree(const GameNode &p_node, const BehaviorSupportPr
   }
   else {
     if (p_node->NumChildren() > 0) {
-      for (size_t i = 1; i <= p_node->NumChildren(); i++) {
-        yn = LayoutSubtree(p_node->GetChild(i), p_support, p_maxy, p_miny, p_ycoord);
+      for (const auto &action : p_node->GetInfoset()->GetActions()) {
+        yn = LayoutSubtree(p_node->GetChild(action), p_support, p_maxy, p_miny, p_ycoord);
         if (y1 == -1) {
           y1 = yn;
         }
 
-        if (!p_node->GetPlayer()->IsChance() &&
-            !p_support.Contains(p_node->GetInfoset()->GetAction(i))) {
+        if (!p_node->GetPlayer()->IsChance() && !p_support.Contains(action)) {
           (*std::find_if(m_nodeList.begin(), m_nodeList.end(), [&](const gbtNodeEntry *e) {
-            return e->GetNode() == p_node->GetChild(i);
+            return e->GetNode() == p_node->GetChild(action);
           }))->SetInSupport(false);
         }
       }

src/pygambit/gambit.pxd

@@ -76,6 +76,7 @@ cdef extern from "games/game.h":
         c_GameActionRep *deref "operator->"() except +RuntimeError
 
     cdef cppclass c_GameInfoset "GameObjectPtr<GameInfosetRep>":
+        bool operator ==(c_GameInfoset) except +
         bool operator !=(c_GameInfoset) except +
         c_GameInfosetRep *deref "operator->"() except +RuntimeError
 
@@ -157,7 +158,7 @@ cdef extern from "games/game.h":
         c_GamePlayer GetPlayer() except +
         c_GameNode GetParent() except +
         int NumChildren() except +
-        c_GameNode GetChild(int) except +IndexError
+        c_GameNode GetChild(c_GameAction) except +IndexError
         c_GameOutcome GetOutcome() except +
         c_GameNode GetPriorSibling() except +
         c_GameNode GetNextSibling() except +

src/pygambit/node.pxi

@@ -42,8 +42,13 @@ class NodeChildren:
         return f"NodeChildren(parent={Node.wrap(self.parent)})"
 
     def __iter__(self) -> typing.Iterator[Node]:
-        for i in range(self.parent.deref().NumChildren()):
-            yield Node.wrap(self.parent.deref().GetChild(i + 1))
+        if self.parent.deref().GetInfoset() != cython.cast(c_GameInfoset, NULL):
+            for i in range(self.parent.deref().GetInfoset().deref().NumActions()):
+                yield Node.wrap(
+                    self.parent.deref().GetChild(
+                        self.parent.deref().GetInfoset().deref().GetAction(i + 1)
+                    )
+                )
 
     def __getitem__(self, index: typing.Union[int, str]) -> Node:
         if isinstance(index, str):
@@ -56,7 +61,11 @@ class NodeChildren:
                 raise ValueError(f"Node has multiple children with label '{index}'")
             return matches[0]
         if isinstance(index, int):
-            return Node.wrap(self.parent.deref().GetChild(index + 1))
+            if self.parent.deref().GetInfoset() == cython.cast(c_GameInfoset, NULL):
+                raise IndexError("Index out of range")
+            return Node.wrap(self.parent.deref().GetChild(
+                self.parent.deref().GetInfoset().deref().GetAction(index + 1)
+            ))
         raise TypeError(f"Child index must be int or str, not {index.__class__.__name__}")