Remove GameRep::Strategies based on dodgy iterator/range.

Author: tturocy

src/games/game.h

@@ -820,19 +820,10 @@ class GameRep : public std::enable_shared_from_this<GameRep> {
 
   /// @name Dimensions of the game
   //@{
-  using Strategies =
-      NestedElementCollection<&GameRep::m_players, &GamePlayerRep::m_strategies, GameStrategy>;
-
   /// The number of strategies for each player
   virtual Array<int> NumStrategies() const = 0;
   /// Gets the i'th strategy in the game, numbered globally
   virtual GameStrategy GetStrategy(int p_index) const = 0;
-  /// Gets the collection of all strategies in the game
-  Strategies GetStrategies() const
-  {
-    BuildComputedValues();
-    return Strategies(this);
-  }
   /// Creates a new strategy for the player
   virtual GameStrategy NewStrategy(const GamePlayer &p_player, const std::string &p_label)
   {

src/games/gameobject.h

@@ -214,124 +214,6 @@ template <class P, class T> class ElementCollection {
   iterator cend() const { return {m_owner, m_container, (m_owner) ? m_container->size() : 0}; }
 };
 
-template <auto OuterMember, auto InnerMember, typename ValueType> class NestedElementCollection {
-  template <typename MemberPtr> struct outer_element_of;
-
-  template <typename OuterClass, typename Elem>
-  struct outer_element_of<std::vector<std::shared_ptr<Elem>> OuterClass::*> {
-    using outer_class = OuterClass;
-    using outer_elem = Elem;
-    using container = std::vector<std::shared_ptr<Elem>>;
-  };
-
-  template <typename MemberPtr> struct inner_element_of;
-
-  template <typename OuterElem, typename InnerElem>
-  struct inner_element_of<std::vector<std::shared_ptr<InnerElem>> OuterElem::*> {
-    using outer_elem = OuterElem;
-    using inner_elem = InnerElem;
-    using container = std::vector<std::shared_ptr<InnerElem>>;
-  };
-
-  using OM = outer_element_of<decltype(OuterMember)>;
-  using IM = inner_element_of<decltype(InnerMember)>;
-
-  using OuterClass = typename OM::outer_class;
-  using OuterElem = typename OM::outer_elem;
-  using InnerElem = typename IM::inner_elem;
-
-public:
-  class iterator {
-    using outer_iter = typename OM::container::const_iterator;
-    using inner_iter = typename IM::container::const_iterator;
-
-    outer_iter m_outer, m_outerEnd;
-    inner_iter m_inner, m_innerEnd;
-    mutable ValueType m_cache;
-
-    void advance()
-    {
-      while (m_outer != m_outerEnd && m_inner == m_innerEnd) {
-        ++m_outer;
-        if (m_outer != m_outerEnd) {
-          const auto &outerPtr = *m_outer;
-          const auto &innerVec = outerPtr.get()->*InnerMember;
-          m_inner = innerVec.begin();
-          m_innerEnd = innerVec.end();
-        }
-      }
-    }
-
-  public:
-    using iterator_category = std::input_iterator_tag;
-    using value_type = ValueType;
-    using difference_type = std::ptrdiff_t;
-    using reference = ValueType;
-    using pointer = const ValueType *;
-
-    iterator() = default;
-
-    iterator(const OuterClass *p_obj, bool p_isEnd)
-    {
-      const auto &outerVec = p_obj->*OuterMember;
-
-      m_outer = outerVec.begin();
-      m_outerEnd = outerVec.end();
-
-      if (p_isEnd || m_outer == m_outerEnd) {
-        m_outer = m_outerEnd;
-        return;
-      }
-
-      const auto &outerPtr = *m_outer;
-      const auto &innerVec = outerPtr.get()->*InnerMember;
-
-      m_inner = innerVec.begin();
-      m_innerEnd = innerVec.end();
-
-      advance();
-    }
-
-    reference operator*() const { return ValueType(*m_inner); }
-
-    pointer operator->() const
-    {
-      m_cache = ValueType(*m_inner);
-      return &m_cache;
-    }
-
-    iterator &operator++()
-    {
-      ++m_inner;
-      advance();
-      return *this;
-    }
-
-    iterator operator++(int)
-    {
-      iterator tmp = *this;
-      ++(*this);
-      return tmp;
-    }
-
-    bool operator==(const iterator &p_other) const
-    {
-      return m_outer == p_other.m_outer && (m_outer == m_outerEnd || m_inner == p_other.m_inner);
-    }
-
-    bool operator!=(const iterator &p_other) const { return !(*this == p_other); }
-  };
-
-private:
-  const OuterClass *m_obj;
-
-public:
-  explicit NestedElementCollection(const OuterClass *p_obj) : m_obj(p_obj) {}
-
-  iterator begin() const { return {m_obj, false}; }
-  iterator end() const { return {m_obj, true}; }
-};
-
 } // end namespace Gambit
 
 #endif // GAMBIT_GAMES_GAMEOBJECT_H

src/solvers/liap/nfgliap.cc

@@ -63,43 +63,45 @@ class StrategicLyapunovFunction : public FunctionOnSimplices {
 inline double sum_player_probs(const MixedStrategyProfile<double> &p_profile,
                                const GamePlayer &p_player)
 {
-  return sum_function(p_player->GetStrategies(),
-                      [&p_profile](const auto &s) -> double { return p_profile[s]; });
+  auto strategies = p_player->GetStrategies();
+  return std::accumulate(
+      strategies.cbegin(), strategies.cend(), 0.0,
+      [p_profile](double t, const GameStrategy &s) { return t + p_profile[s]; });
 }
 
 double StrategicLyapunovFunction::Value(const Vector<double> &v) const
 {
   m_profile = v;
   double value = 0;
-  // Liapunov function proper
-  for (const auto &player : m_profile.GetGame()->GetPlayers()) {
-    const auto payoff = m_profile.GetPayoff(player);
-    for (const auto strategy : player->GetStrategies()) {
-      value += sqr(std::max(m_scale * (m_profile.GetPayoff(strategy) - payoff), 0.0));
+  // Liapunov function proper - should be replaced with call to profile once
+  // the penalty is removed from that implementation.
+  for (auto player : m_profile.GetGame()->GetPlayers()) {
+    for (auto strategy : player->GetStrategies()) {
+      value += sqr(
+          std::max(m_scale * (m_profile.GetPayoff(strategy) - m_profile.GetPayoff(player)), 0.0));
     }
   }
   // Penalty function for non-negativity constraint for each strategy
-  value += m_penalty *
-           sum_function(m_profile.GetGame()->GetStrategies(), [this](const auto &s) -> double {
-             return sqr(std::min(m_profile[s], 0.0));
-           });
-
+  for (auto player : m_profile.GetGame()->GetPlayers()) {
+    for (auto strategy : player->GetStrategies()) {
+      value += m_penalty * sqr(std::min(m_profile[strategy], 0.0));
+    }
+  }
   // Penalty function for sum-to-one constraint for each player
-  value +=
-      m_penalty * sum_function(m_profile.GetGame()->GetPlayers(), [this](const auto &p) -> double {
-        return sqr(sum_player_probs(m_profile, p) - 1.0);
-      });
+  for (auto player : m_profile.GetGame()->GetPlayers()) {
+    value += m_penalty * sqr(sum_player_probs(m_profile, player) - 1.0);
+  }
   return value;
 }
 
 double StrategicLyapunovFunction::LiapDerivValue(const MixedStrategyProfile<double> &p_profile,
                                                  const GameStrategy &p_wrt_strategy) const
 {
   double deriv = 0.0;
-  for (const auto &player : m_game->GetPlayers()) {
-    const auto payoff = p_profile.GetPayoff(player);
-    for (const auto &strategy : player->GetStrategies()) {
-      const double loss = sqr(m_scale) * (p_profile.GetPayoff(strategy) - payoff);
+  for (auto player : m_game->GetPlayers()) {
+    for (auto strategy : player->GetStrategies()) {
+      const double loss =
+          sqr(m_scale) * (p_profile.GetPayoff(strategy) - p_profile.GetPayoff(player));
       if (loss <= 0.0) {
         continue;
       }
@@ -135,8 +137,10 @@ namespace {
 MixedStrategyProfile<double> EnforceNonnegativity(const MixedStrategyProfile<double> &p_profile)
 {
   auto profile = p_profile;
-  for (const auto &strategy : p_profile.GetGame()->GetStrategies()) {
-    profile[strategy] = std::max(profile[strategy], 0.0);
+  for (auto player : p_profile.GetGame()->GetPlayers()) {
+    for (auto strategy : player->GetStrategies()) {
+      profile[strategy] = std::max(profile[strategy], 0.0);
+    }
   }
   return profile.Normalize();
 }