Refactor GameTreeRep::IsPerfectRecall with efficient single-pass algorithm; add new tests

src/games/game.h

@@ -412,12 +412,29 @@ class GameNodeRep : public std::enable_shared_from_this<GameNodeRep> {
 public:
   using Children = ElementCollection<GameNode, GameNodeRep>;
 
+  /// @brief A range class for iterating over a node's (action, child) pairs.
+  class Actions {
+  private:
+    const GameNodeRep *m_owner{nullptr};
+
+  public:
+    class iterator;
+
+    Actions(const GameNodeRep *p_owner);
+
+    iterator begin() const;
+    iterator end() const;
+  };
+
   GameNodeRep(GameRep *e, GameNodeRep *p);
   ~GameNodeRep();
 
   bool IsValid() const { return m_valid; }
   void Invalidate() { m_valid = false; }
 
+  /// @brief Returns a collection for iterating over this node's (action, child) pairs.
+  Actions GetActions() const;
+
   Game GetGame() const;
 
   const std::string &GetLabel() const { return m_label; }
@@ -451,6 +468,97 @@ class GameNodeRep : public std::enable_shared_from_this<GameNodeRep> {
   bool IsSubgameRoot() const;
 };
 
+class GameNodeRep::Actions::iterator {
+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.
+  iterator() = default;
+
+  /// Creates a new iterator that zips an action iterator and a child iterator.
+  iterator(GameInfosetRep::Actions::iterator p_action_it,
+           GameNodeRep::Children::iterator 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).
+  iterator &operator++()
+  {
+    ++m_action_it;
+    ++m_child_it;
+    return *this;
+  }
+
+  /// Advances the iterator to the next pair (post-increment).
+  iterator operator++(int)
+  {
+    iterator tmp = *this;
+    ++(*this);
+    return tmp;
+  }
+
+  /// Compares two iterators for equality.
+  bool operator==(const iterator &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 iterator &p_other) const { return !(*this == p_other); }
+  //@}
+
+  GameNode GetOwner() const;
+};
+
+inline GameNodeRep::Actions::Actions(const GameNodeRep *p_owner) : m_owner(p_owner) {}
+
+inline GameNodeRep::Actions GameNodeRep::GetActions() const { return {Actions(this)}; }
+
+inline GameNodeRep::Actions::iterator GameNodeRep::Actions::begin() const
+{
+  if (m_owner->IsTerminal()) {
+    return end();
+  }
+  return {m_owner->GetInfoset()->GetActions().begin(), m_owner->GetChildren().begin()};
+}
+
+inline GameNodeRep::Actions::iterator GameNodeRep::Actions::end() const
+{
+  if (m_owner->IsTerminal()) {
+    return {};
+  }
+  return {m_owner->GetInfoset()->GetActions().end(), m_owner->GetChildren().end()};
+}
+
+inline GameNodeRep::Actions::iterator::iterator(GameInfosetRep::Actions::iterator p_action_it,
+                                                GameNodeRep::Children::iterator p_child_it)
+  : m_action_it(p_action_it), m_child_it(p_child_it)
+{
+}
+
+inline GameNode GameNodeRep::Actions::iterator::GetOwner() const { return m_child_it.GetOwner(); }
+
 /// This is the class for representing an arbitrary finite game.
 class GameRep : public std::enable_shared_from_this<GameRep> {
   friend class GameOutcomeRep;
@@ -611,15 +719,8 @@ class GameRep : public std::enable_shared_from_this<GameRep> {
   /// Returns the set of terminal nodes which are descendants of members of an action
   virtual std::vector<GameNode> GetPlays(GameAction action) const { throw UndefinedException(); }
 
-  /// Returns true if the game is perfect recall.  If not,
-  /// a pair of violating information sets is returned in the parameters.
-  virtual bool IsPerfectRecall(GameInfoset &, GameInfoset &) const = 0;
   /// Returns true if the game is perfect recall
-  bool IsPerfectRecall() const
-  {
-    GameInfoset s, t;
-    return IsPerfectRecall(s, t);
-  }
+  virtual bool IsPerfectRecall() const = 0;
   //@}
 
   /// @name Writing data files

src/games/gameagg.h

@@ -84,7 +84,7 @@ class GameAGGRep : public GameRep {
   //@{
   bool IsTree() const override { return false; }
   bool IsAgg() const override { return true; }
-  bool IsPerfectRecall(GameInfoset &, GameInfoset &) const override { return true; }
+  bool IsPerfectRecall() const override { return true; }
   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()); }

src/games/gamebagg.h

@@ -91,7 +91,7 @@ class GameBAGGRep : public GameRep {
   //@{
   bool IsTree() const override { return false; }
   virtual bool IsBagg() const { return true; }
-  bool IsPerfectRecall(GameInfoset &, GameInfoset &) const override { return true; }
+  bool IsPerfectRecall() const override { return true; }
   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()); }

src/games/gameobject.h

@@ -141,6 +141,8 @@ template <class P, class T> class ElementCollection {
       return *this;
     }
     value_type operator*() const { return m_container->at(m_index); }
+
+    inline const P &GetOwner() const { return m_owner; }
   };
 
   ElementCollection() = default;

src/games/gametable.h

@@ -57,7 +57,7 @@ class GameTableRep : public GameExplicitRep {
   //@{
   bool IsTree() const override { return false; }
   bool IsConstSum() const override;
-  bool IsPerfectRecall(GameInfoset &, GameInfoset &) const override { return true; }
+  bool IsPerfectRecall() const override { return true; }
   //@}
 
   /// @name Dimensions of the game

src/games/gametree.cc

@@ -23,6 +23,9 @@
 #include <iostream>
 #include <algorithm>
 #include <numeric>
+#include <stack>
+#include <set>
+#include <variant>
 
 #include "gambit.h"
 #include "gametree.h"
@@ -747,54 +750,18 @@ bool GameTreeRep::IsConstSum() const
   }
 }
 
-bool GameTreeRep::IsPerfectRecall(GameInfoset &s1, GameInfoset &s2) const
+bool GameTreeRep::IsPerfectRecall() const
 {
-  for (auto player : m_players) {
-    for (size_t i = 1; i <= player->m_infosets.size(); i++) {
-      auto iset1 = player->m_infosets[i - 1];
-      for (size_t j = 1; j <= player->m_infosets.size(); j++) {
-        auto iset2 = player->m_infosets[j - 1];
-
-        bool precedes = false;
-        GameAction action = nullptr;
-
-        for (size_t m = 1; m <= iset2->m_members.size(); m++) {
-          size_t n;
-          for (n = 1; n <= iset1->m_members.size(); n++) {
-            if (iset2->GetMember(m)->IsSuccessorOf(iset1->GetMember(n)) &&
-                iset1->GetMember(n) != iset2->GetMember(m)) {
-              precedes = true;
-              for (const auto &act : iset1->GetActions()) {
-                if (iset2->GetMember(m)->IsSuccessorOf(iset1->GetMember(n)->GetChild(act))) {
-                  if (action != nullptr && action != act) {
-                    s1 = iset1;
-                    s2 = iset2;
-                    return false;
-                  }
-                  action = act;
-                }
-              }
-              break;
-            }
-          }
-
-          if (i == j && precedes) {
-            s1 = iset1;
-            s2 = iset2;
-            return false;
-          }
+  if (m_infosetParents.empty() && !m_root->IsTerminal()) {
+    const_cast<GameTreeRep *>(this)->BuildInfosetParents();
+  }
 
-          if (n > iset1->m_members.size() && precedes) {
-            s1 = iset1;
-            s2 = iset2;
-            return false;
-          }
-        }
-      }
-    }
+  if (GetRoot()->IsTerminal()) {
+    return true;
   }
 
-  return true;
+  return std::all_of(m_infosetParents.cbegin(), m_infosetParents.cend(),
+                     [](const auto &pair) { return pair.second.size() <= 1; });
 }
 
 //------------------------------------------------------------------------
@@ -871,6 +838,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;
 }
 
@@ -911,6 +879,77 @@ std::vector<GameNodeRep *> GameTreeRep::BuildConsistentPlaysRecursiveImpl(GameNo
   return consistent_plays;
 }
 
+void GameTreeRep::BuildInfosetParents()
+{
+  if (m_root->IsTerminal()) {
+    m_infosetParents[m_root->m_infoset].insert(nullptr);
+    return;
+  }
+
+  using AbsentMindedEdge = std::pair<GameAction, GameNode>;
+  using ActiveEdge = std::variant<GameNodeRep::Actions::iterator, AbsentMindedEdge>;
+  std::stack<ActiveEdge> position;
+
+  std::map<GamePlayer, std::stack<GameAction>> prior_actions;
+  std::map<GameInfoset, GameAction> path_choices;
+
+  for (auto player_rep : m_players) {
+    prior_actions[GamePlayer(player_rep)].emplace(nullptr);
+  }
+  prior_actions[GamePlayer(m_chance)].emplace(nullptr);
+
+  position.emplace(m_root->GetActions().begin());
+  prior_actions[m_root->m_infoset->m_player->shared_from_this()].emplace(nullptr);
+  if (m_root->m_infoset) {
+    m_infosetParents[m_root->m_infoset].insert(nullptr);
+  }
+
+  while (!position.empty()) {
+    ActiveEdge &current_edge = position.top();
+    GameNode child, node;
+    GameAction action;
+
+    if (std::holds_alternative<GameNodeRep::Actions::iterator>(current_edge)) {
+      auto &current_it = std::get<GameNodeRep::Actions::iterator>(current_edge);
+      node = current_it.GetOwner();
+
+      if (current_it == node->GetActions().end()) {
+        prior_actions.at(node->m_infoset->m_player->shared_from_this()).pop();
+        position.pop();
+        path_choices.erase(node->m_infoset->shared_from_this());
+        continue;
+      }
+      else {
+        std::tie(action, child) = *current_it;
+        ++current_it;
+        path_choices[node->m_infoset->shared_from_this()] = action;
+      }
+    }
+    else {
+      std::tie(action, node) = std::get<AbsentMindedEdge>(current_edge);
+      position.pop();
+      child = node->GetChild(action);
+    }
+
+    prior_actions.at(node->m_infoset->m_player->shared_from_this()).top() = action;
+
+    if (!child->IsTerminal()) {
+      auto child_player = child->m_infoset->m_player->shared_from_this();
+      auto prior_action = prior_actions.at(child_player).top();
+      m_infosetParents[child->m_infoset].insert(prior_action ? prior_action.get() : nullptr);
+
+      if (path_choices.find(child->m_infoset->shared_from_this()) != path_choices.end()) {
+        const GameAction replay_action = path_choices.at(child->m_infoset->shared_from_this());
+        position.emplace(AbsentMindedEdge{replay_action, child});
+      }
+      else {
+        position.emplace(child->GetActions().begin());
+      }
+      prior_actions.at(child_player).emplace(nullptr);
+    }
+  }
+}
+
 //------------------------------------------------------------------------
 //                  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<GameInfosetRep *, std::set<GameActionRep *>> m_infosetParents;
 
   /// @name Private auxiliary functions
   //@{
@@ -72,8 +73,7 @@ class GameTreeRep : public GameExplicitRep {
   //@{
   bool IsTree() const override { return true; }
   bool IsConstSum() const override;
-  using GameRep::IsPerfectRecall;
-  bool IsPerfectRecall(GameInfoset &, GameInfoset &) const override;
+  bool IsPerfectRecall() const override;
   /// Turn on or off automatic canonicalization of the game
   void SetCanonicalization(bool p_doCanon) const
   {
@@ -160,6 +160,7 @@ class GameTreeRep : public GameExplicitRep {
 
 private:
   std::vector<GameNodeRep *> BuildConsistentPlaysRecursiveImpl(GameNodeRep *node);
+  void BuildInfosetParents();
 };
 
 template <class T> class TreeMixedStrategyProfileRep : public MixedStrategyProfileRep<T> {