Add ActionsIterator class to zip actions and children; refactor PR-checker using a separate BuildInfosetParents method

Author: d-kad

src/games/game.cc

@@ -188,6 +188,20 @@ size_t GamePlayerRep::NumSequences() const
                                [](const GameInfosetRep *s) { return s->m_actions.size(); });
 }
 
+//========================================================================
+//                            class GameNodeRep
+//========================================================================
+
+GameNodeRep::Actions::iterator GameNodeRep::Actions::begin() const
+{
+  return {m_owner->GetInfoset()->GetActions().begin(), m_owner->GetChildren().begin()};
+}
+
+GameNodeRep::Actions::iterator GameNodeRep::Actions::end() const
+{
+  return {m_owner->GetInfoset()->GetActions().end(), m_owner->GetChildren().end()};
+}
+
 //========================================================================
 //                            class GameRep
 //========================================================================

src/games/game.h

@@ -55,6 +55,8 @@ using GameNode = GameObjectPtr<GameNodeRep>;
 class GameRep;
 using Game = GameObjectPtr<GameRep>;
 
+class ActionsIterator;
+
 template <class P, class T> class ElementCollection {
   P m_owner{nullptr};
   const std::vector<T *> *m_container{nullptr};
@@ -103,6 +105,7 @@ template <class P, class T> class ElementCollection {
       return *this;
     }
     value_type operator*() const { return m_container->at(m_index); }
+    P GetOwner() const { return m_owner; }
   };
 
   ElementCollection() = default;
@@ -454,6 +457,23 @@ class GameNodeRep : public GameObject {
 public:
   using Children = ElementCollection<GameNode, GameNodeRep>;
 
+  /// @brief A range class for iterating over a node's (action, child) pairs.
+  class Actions {
+  private:
+    GameNode m_owner{nullptr};
+
+  public:
+    using iterator = ActionsIterator;
+
+    explicit Actions(GameNode p_owner) : m_owner(p_owner) {}
+
+    iterator begin() const;
+    iterator end() const;
+  };
+
+  /// @brief Returns a range for iterating over this node's (action, child) pairs.
+  Actions GetActions() const { return Actions(this); }
+
   Game GetGame() const;
 
   const std::string &GetLabel() const { return m_label; }
@@ -484,6 +504,76 @@ class GameNodeRep : public GameObject {
   bool IsSubgameRoot() const;
 };
 
+class ActionsIterator {
+public:
+  /// @name Iterator
+  //@{
+  using iterator_category = std::forward_iterator_tag;
+  using value_type = std::pair<GameAction, GameNode>;
+  using difference_type = std::ptrdiff_t;
+  using pointer = value_type *;
+  using reference = value_type;
+  //@}
+
+private:
+  /// @brief An iterator to the action at the parent's information set.
+  GameInfosetRep::Actions::iterator m_action_it;
+  /// @brief An iterator to the child node.
+  GameNodeRep::Children::iterator m_child_it;
+
+public:
+  /// @name Lifecycle
+  //@{
+  /// Default constructor. Creates an iterator in a past-the-end state.
+  ActionsIterator() = default;
+
+  /// Creates a new iterator that zips an action iterator and a child iterator.
+  ActionsIterator(GameInfosetRep::Actions::iterator p_action_it,
+                  GameNodeRep::Children::iterator p_child_it)
+    : m_action_it(p_action_it), m_child_it(p_child_it)
+  {
+  }
+  //@}
+
+  /// @name Iterator Operations
+  //@{
+  /// Returns the current action-child pair.
+  reference operator*() const { return {*m_action_it, *m_child_it}; }
+
+  /// Advances the iterator to the next pair (pre-increment).
+  ActionsIterator &operator++()
+  {
+    ++m_action_it;
+    ++m_child_it;
+    return *this;
+  }
+
+  /// Advances the iterator to the next pair (post-increment).
+  ActionsIterator operator++(int)
+  {
+    ActionsIterator tmp = *this;
+    ++(*this);
+    return tmp;
+  }
+
+  /// Compares two iterators for equality.
+  bool operator==(const ActionsIterator &p_other) const
+  {
+    // Comparing one of the wrapped iterators is sufficient as they move in lockstep.
+    return m_child_it == p_other.m_child_it;
+  }
+
+  /// Compares two iterators for inequality.
+  bool operator!=(const ActionsIterator &p_other) const { return !(*this == p_other); }
+  //@}
+
+  GameNode GetOwner() const
+  {
+    // Delegate the request to the internal child iterator.
+    return m_child_it.GetOwner();
+  }
+};
+
 /// This is the class for representing an arbitrary finite game.
 class GameRep : public BaseGameRep {
   friend class GameOutcomeRep;

src/games/gametree.cc

@@ -736,61 +736,21 @@ bool GameTreeRep::IsConstSum() const
 
 bool GameTreeRep::IsPerfectRecall() const
 {
-  using ChildIterator = GameNodeRep::Children::iterator;
-  using ActionIterator = GameInfosetRep::Actions::iterator;
-
-  std::map<GamePlayer, std::stack<GameAction>> prior_actions;
-  std::map<GameInfoset, std::vector<GameAction>> infoset_parents;
-  std::stack<std::tuple<GameNode, ChildIterator, ActionIterator>> position;
-
-  for (auto player : GetPlayers()) {
-    prior_actions[player].emplace(nullptr);
+  if (m_infosetParents.empty() && !GetRoot()->IsTerminal()) {
+    const_cast<GameTreeRep *>(this)->BuildInfosetParents();
   }
-  prior_actions[GetChance()].emplace(nullptr);
-  prior_actions[GetRoot()->GetPlayer()].emplace(nullptr);
 
   if (GetRoot()->IsTerminal()) {
     return true;
   }
 
-  position.emplace(GetRoot(), GetRoot()->GetChildren().begin(),
-                   GetRoot()->GetInfoset()->GetActions().begin());
-
-  infoset_parents[GetRoot()->GetInfoset()].emplace_back(nullptr);
-
-  while (!position.empty()) {
-    auto &[parent, child_it, action_it] = position.top();
-
-    if (child_it != parent->GetChildren().end()) {
-      const GameNode child = *child_it;
-      const GameAction action = *action_it;
-
-      prior_actions[parent->GetPlayer()].top() = action;
-
-      if (!child->IsTerminal()) {
-        infoset_parents[child->GetInfoset()].push_back(prior_actions[child->GetPlayer()].top());
-        position.emplace(child, child->GetChildren().begin(),
-                         child->GetInfoset()->GetActions().begin());
-        prior_actions[child->GetPlayer()].emplace(nullptr);
-      }
-      ++child_it;
-      ++action_it;
-    }
-
-    else {
-      prior_actions[parent->GetPlayer()].pop();
-      position.pop();
-    }
-  }
-
-  for (const auto &[infoset, parent_action_options] : infoset_parents) {
+  for (const auto &[infoset, parent_action_options] : m_infosetParents) {
     const std::set<GameAction> unique_parents(parent_action_options.begin(),
                                               parent_action_options.end());
     if (unique_parents.size() > 1) {
       return false;
     }
   }
-
   return true;
 }
 
@@ -867,6 +827,7 @@ void GameTreeRep::ClearComputedValues() const
     player->m_strategies.clear();
   }
   const_cast<GameTreeRep *>(this)->m_nodePlays.clear();
+  const_cast<GameTreeRep *>(this)->m_infosetParents.clear();
   m_computedValues = false;
 }
 
@@ -905,6 +866,43 @@ std::vector<GameNodeRep *> GameTreeRep::BuildConsistentPlaysRecursiveImpl(GameNo
   return consistent_plays;
 }
 
+void GameTreeRep::BuildInfosetParents()
+{
+  std::map<GamePlayer, std::stack<GameAction>> prior_actions;
+  std::stack<ActionsIterator> position;
+
+  for (auto player : GetPlayers()) {
+    prior_actions[player].emplace(nullptr);
+  }
+  prior_actions[GetChance()].emplace(nullptr);
+  prior_actions[GetRoot()->GetPlayer()].emplace(nullptr);
+
+  position.emplace(GetRoot()->GetActions().begin());
+  m_infosetParents[GetRoot()->GetInfoset()].emplace_back(nullptr);
+
+  while (!position.empty()) {
+    ActionsIterator &current_it = position.top();
+    const GameNode parent = current_it.GetOwner();
+
+    if (current_it != parent->GetActions().end()) {
+      auto [action, child] = *current_it;
+
+      prior_actions[parent->GetPlayer()].top() = action;
+
+      if (!child->IsTerminal()) {
+        m_infosetParents[child->GetInfoset()].push_back(prior_actions[child->GetPlayer()].top());
+        position.emplace(child->GetActions().begin());
+        prior_actions[child->GetPlayer()].emplace(nullptr);
+      }
+      ++current_it;
+    }
+    else {
+      prior_actions[parent->GetPlayer()].pop();
+      position.pop();
+    }
+  }
+}
+
 //------------------------------------------------------------------------
 //                  GameTreeRep: Writing data files
 //------------------------------------------------------------------------

src/games/gametree.h

@@ -39,6 +39,7 @@ class GameTreeRep : public GameExplicitRep {
   std::size_t m_numNodes = 1;
   std::size_t m_numNonterminalNodes = 0;
   std::map<GameNodeRep *, std::vector<GameNodeRep *>> m_nodePlays;
+  std::map<GameInfoset, std::vector<GameAction>> m_infosetParents;
 
   /// @name Private auxiliary functions
   //@{
@@ -159,6 +160,7 @@ class GameTreeRep : public GameExplicitRep {
 
 private:
   std::vector<GameNodeRep *> BuildConsistentPlaysRecursiveImpl(GameNodeRep *node);
+  void BuildInfosetParents();
 };
 
 template <class T> class TreeMixedStrategyProfileRep : public MixedStrategyProfileRep<T> {