From 2ce5af654183a70ccd8b32773f668cd89f6893eb Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 13:18:44 +0000 Subject: [PATCH 1/3] Some style improvements to Array. --- src/core/array.h | 64 +++++++++++++++++++++------------ src/gui/nfgtable.cc | 4 +-- src/solvers/linalg/lemketab.imp | 4 +-- src/solvers/linalg/lptab.imp | 14 ++++---- 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/core/array.h b/src/core/array.h index 82836db5b..57ed51936 100644 --- a/src/core/array.h +++ b/src/core/array.h @@ -39,6 +39,11 @@ template class Array { int m_offset; std::vector m_data; + bool is_in_range(const int p_index) const + { + return p_index >= m_offset && p_index <= back_index(); + } + public: using iterator = typename std::vector::iterator; using const_iterator = typename std::vector::const_iterator; @@ -46,31 +51,43 @@ template class Array { using reference = typename std::vector::reference; using const_reference = typename std::vector::const_reference; - explicit Array(size_t len = 0) : m_offset(1), m_data(len) {} - Array(int lo, int hi) : m_offset(lo), m_data(hi - lo + 1) {} - Array(const Array &) = default; + explicit Array(size_t p_length = 0) : m_offset(1), m_data(p_length) {} + /// Construct an array with the given index bounds. + /// If p_high is less than p_low, the resulting array will have + /// front_index equal to p_low, and size 0. + explicit Array(const int p_low, const int p_high) : m_offset(p_low) + { + if (p_high >= p_low) { + m_data.resize(p_high - p_low + 1); + } + } + Array(const Array &) = default; + Array(Array &&) noexcept = default; ~Array() = default; - Array &operator=(const Array &) = default; - - bool operator==(const Array &a) const { return m_offset == a.m_offset && m_data == a.m_data; } + Array &operator=(const Array &) = default; + Array &operator=(Array &&) noexcept = default; - bool operator!=(const Array &a) const { return m_offset != a.m_offset || m_data != a.m_data; } + bool operator==(const Array &p_other) const + { + return m_offset == p_other.m_offset && m_data == p_other.m_data; + } + bool operator!=(const Array &p_other) const { return !(*this == p_other); } - const_reference operator[](int index) const + const_reference operator[](const int p_index) const { - if (index < m_offset || index > back_index()) { + if (!is_in_range(p_index)) { throw std::out_of_range("Index out of range in Array"); } - return m_data.at(index - m_offset); + return m_data.at(p_index - m_offset); } - reference operator[](int index) + reference operator[](const int p_index) { - if (index < m_offset || index > back_index()) { + if (!is_in_range(p_index)) { throw std::out_of_range("Index out of range in Array"); } - return m_data[index - m_offset]; + return m_data.at(p_index - m_offset); } const T &front() const { return m_data.front(); } T &front() { return m_data.front(); } @@ -84,25 +101,26 @@ template class Array { const_iterator cbegin() const { return m_data.cbegin(); } const_iterator cend() const { return m_data.cend(); } - bool empty() const { return m_data.empty(); } - size_t size() const { return m_data.size(); } - int front_index() const { return m_offset; } - int back_index() const { return m_offset + m_data.size() - 1; } + [[nodiscard]] bool empty() const noexcept { return m_data.empty(); } + [[nodiscard]] size_t size() const noexcept { return m_data.size(); } + [[nodiscard]] int front_index() const noexcept { return m_offset; } + [[nodiscard]] int back_index() const { return m_offset + m_data.size() - 1; } void clear() { m_data.clear(); } void insert(const_iterator pos, const T &value) { m_data.insert(pos, value); } void erase(iterator pos) { m_data.erase(pos); } + void erase_at(const int p_index) + { + if (!is_in_range(p_index)) { + throw std::out_of_range("Index out of range in Array"); + } + erase(std::next(begin(), p_index - front_index())); + } void push_back(const T &value) { m_data.push_back(value); } void pop_back() { m_data.pop_back(); } void reserve(size_t len) { m_data.reserve(len); } }; -/// Convenience function to erase the element at `p_index` -template void erase_atindex(Array &p_array, int p_index) -{ - p_array.erase(std::next(p_array.begin(), p_index - p_array.front_index())); -} - } // end namespace Gambit #endif // GAMBIT_CORE_ARRAY_H diff --git a/src/gui/nfgtable.cc b/src/gui/nfgtable.cc index 9a7d3756a..77bdd866a 100644 --- a/src/gui/nfgtable.cc +++ b/src/gui/nfgtable.cc @@ -1021,13 +1021,13 @@ void TableWidget::OnUpdate() else if (m_doc->NumPlayers() < m_rowPlayers.size() + m_colPlayers.size()) { for (size_t i = 1; i <= m_rowPlayers.size(); i++) { if (m_rowPlayers[i] > static_cast(m_doc->NumPlayers())) { - erase_atindex(m_rowPlayers, i--); + m_rowPlayers.erase_at(i--); } } for (size_t i = 1; i <= m_colPlayers.size(); i++) { if (m_colPlayers[i] > static_cast(m_doc->NumPlayers())) { - erase_atindex(m_colPlayers, i--); + m_colPlayers.erase_at(i--); } } } diff --git a/src/solvers/linalg/lemketab.imp b/src/solvers/linalg/lemketab.imp index 99f0c1911..bdab4845c 100644 --- a/src/solvers/linalg/lemketab.imp +++ b/src/solvers/linalg/lemketab.imp @@ -88,7 +88,7 @@ template int LemkeTableau::SF_ExitIndex(int inlabel) for (size_t i = BestSet.size(); i >= 1; i--) { ratio = col[BestSet[i]] / incol[BestSet[i]]; if (ratio > tempmax + this->eps2) { - erase_atindex(BestSet, i); + BestSet.erase_at(i); } } c++; @@ -159,7 +159,7 @@ template int LemkeTableau::ExitIndex(int inlabel) for (size_t j = BestSet.size(); j >= 1; j--) { ratio = col[BestSet[j]] / incol[BestSet[j]]; if (ratio < tempmax - this->eps1) { - erase_atindex(BestSet, j); + BestSet.erase_at(j); } } c++; diff --git a/src/solvers/linalg/lptab.imp b/src/solvers/linalg/lptab.imp index 764d47fd2..c9835a783 100644 --- a/src/solvers/linalg/lptab.imp +++ b/src/solvers/linalg/lptab.imp @@ -180,7 +180,7 @@ template std::list> LPTableau::ReversePivots() for (int i = best_set.size(); i >= 1; i--) { x = solution[best_set[i]] / tmpcol[best_set[i]]; if (this->LtZero(x - ratio)) { - erase_atindex(best_set, i); + best_set.erase_at(i); } } @@ -190,7 +190,7 @@ template std::list> LPTableau::ReversePivots() for (int i = best_set.size(); i >= 1; i--) { a_ij = static_cast(1) / tmpcol[best_set[i]]; if (this->LeZero(a_ij)) { - erase_atindex(best_set, i); + best_set.erase_at(i); } else { // next check that prior pivot entry attains max ratio @@ -203,12 +203,12 @@ template std::list> LPTableau::ReversePivots() a_ik = -a_ij * tmpcol[k]; b_k = solution[k] - b_i * tmpcol[k]; if (this->GtZero(a_ik) && this->GtZero(b_k / a_ik - ratio)) { - erase_atindex(best_set, i); + best_set.erase_at(i); flag = true; } else if (this->GtZero(a_ik) && this->EqZero(b_k / a_ik - ratio) && this->Label(k) < j) { - erase_atindex(best_set, i); + best_set.erase_at(i); flag = true; } } @@ -227,7 +227,7 @@ template std::list> LPTableau::ReversePivots() a_ij = tmpdual * tmpcol; c_j = RelativeCost(j); if (this->EqZero(a_ij)) { - erase_atindex(best_set, i); + best_set.erase_at(i); } else { ratio = c_j / a_ij; @@ -241,7 +241,7 @@ template std::list> LPTableau::ReversePivots() c_k = RelativeCost(enter); c_jo = c_k - a_ik * ratio; if (this->GeZero(c_jo)) { - erase_atindex(best_set, i); + best_set.erase_at(i); } else { flag = false; @@ -258,7 +258,7 @@ template std::list> LPTableau::ReversePivots() c_jo = c_k - a_ik * ratio; if (this->LtZero(c_jo)) { - erase_atindex(best_set, i); + best_set.erase_at(i); flag = true; } } From bf6e207032e9bebbaf8e13f5463649e35638ed59 Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 13:38:33 +0000 Subject: [PATCH 2/3] Some style improvements to Vector. --- src/core/vector.h | 100 +++++++++++++++++++++++----------------------- 1 file changed, 51 insertions(+), 49 deletions(-) diff --git a/src/core/vector.h b/src/core/vector.h index 66a24f34a..0b63014f6 100644 --- a/src/core/vector.h +++ b/src/core/vector.h @@ -35,44 +35,39 @@ template class Matrix; template class Vector { Array m_data; - // check vector for identical boundaries - bool Check(const Vector &v) const + bool is_conformable(const Vector &p_other) const { - return v.front_index() == front_index() && v.size() == size(); + return p_other.front_index() == front_index() && p_other.size() == size(); } public: - using iterator = typename std::vector::iterator; - using const_iterator = typename std::vector::const_iterator; + using iterator = typename Array::iterator; + using const_iterator = typename Array::const_iterator; - /// Create a vector of length len, starting at 1 explicit Vector(size_t len = 0) : m_data(len) {} - /// Create a vector indexed from low to high Vector(int low, int high) : m_data(low, high) {} - /// Copy constructor - Vector(const Vector &) = default; - /// Destructor + Vector(Vector &&) noexcept = default; + Vector(const Vector &) = default; ~Vector() = default; - /// Assignment operator: requires vectors to have same index range - Vector &operator=(const Vector &v) + Vector &operator=(Vector &&v) noexcept = default; + Vector &operator=(const Vector &v) { - if (!Check(v)) { + if (!is_conformable(v)) { throw DimensionException(); } m_data = v.m_data; return *this; } - /// Assigns the value c to all components of the vector - Vector &operator=(const T &c) + Vector &operator=(const T &c) { std::fill(m_data.begin(), m_data.end(), c); return *this; } - int front_index() const { return m_data.front_index(); } - int back_index() const { return m_data.back_index(); } - size_t size() const { return m_data.size(); } + int front_index() const noexcept { return m_data.front_index(); } + int back_index() const noexcept { return m_data.back_index(); } + size_t size() const noexcept { return m_data.size(); } const T &front() const { return m_data.front(); } T &front() { return m_data.front(); } @@ -89,20 +84,20 @@ template class Vector { const_iterator cbegin() const { return m_data.cbegin(); } const_iterator cend() const { return m_data.cend(); } - Vector operator+(const Vector &v) const + Vector operator+(const Vector &v) const { - if (!Check(v)) { + if (!is_conformable(v)) { throw DimensionException(); } - Vector tmp(front_index(), back_index()); + Vector tmp(front_index(), back_index()); std::transform(m_data.cbegin(), m_data.cend(), v.m_data.cbegin(), tmp.m_data.begin(), std::plus<>()); return tmp; } - Vector &operator+=(const Vector &v) + Vector &operator+=(const Vector &v) { - if (!Check(v)) { + if (!is_conformable(v)) { throw DimensionException(); } std::transform(m_data.cbegin(), m_data.cend(), v.m_data.cbegin(), m_data.begin(), @@ -110,20 +105,20 @@ template class Vector { return *this; } - Vector operator-(const Vector &v) const + Vector operator-(const Vector &v) const { - if (!Check(v)) { + if (!is_conformable(v)) { throw DimensionException(); } - Vector tmp(front_index(), back_index()); + Vector tmp(front_index(), back_index()); std::transform(m_data.cbegin(), m_data.cend(), v.m_data.cbegin(), tmp.m_data.begin(), std::minus<>()); return tmp; } - Vector &operator-=(const Vector &v) + Vector &operator-=(const Vector &v) { - if (!Check(v)) { + if (!is_conformable(v)) { throw DimensionException(); } std::transform(m_data.cbegin(), m_data.cend(), v.m_data.cbegin(), m_data.begin(), @@ -131,64 +126,71 @@ template class Vector { return *this; } - Vector operator*(const T &c) const + Vector operator*(const T &c) const { - Vector tmp(front_index(), back_index()); + Vector tmp(front_index(), back_index()); std::transform(m_data.cbegin(), m_data.cend(), tmp.m_data.begin(), - [&](const T &v) { return v * c; }); + [&c](const T &v) { return v * c; }); return tmp; } - Vector &operator*=(const T &c) + Vector &operator*=(const T &c) { - std::transform(m_data.cbegin(), m_data.cend(), m_data.begin(), - [&](const T &v) { return v * c; }); + std::transform(m_data.begin(), m_data.end(), m_data.begin(), + [&c](const T &v) { return v * c; }); return *this; } - T operator*(const Vector &v) const + T operator*(const Vector &v) const { - if (!Check(v)) { + if (!is_conformable(v)) { throw DimensionException(); } - return std::inner_product(m_data.begin(), m_data.end(), v.m_data.begin(), static_cast(0)); + return std::inner_product(m_data.cbegin(), m_data.cend(), v.m_data.cbegin(), + static_cast(0)); } - Vector operator/(const T &c) const + Vector operator/(const T &c) const { - Vector tmp(front_index(), back_index()); + Vector tmp(front_index(), back_index()); std::transform(m_data.cbegin(), m_data.cend(), tmp.m_data.begin(), - [&](const T &v) { return v / c; }); + [&c](const T &v) { return v / c; }); return tmp; } - Vector &operator/=(const T &c) + Vector &operator/=(const T &c) { std::transform(m_data.cbegin(), m_data.cend(), m_data.begin(), - [&](const T &v) { return v / c; }); + [&c](const T &v) { return v / c; }); return *this; } - bool operator==(const Vector &v) const { return m_data == v.m_data; } - bool operator!=(const Vector &v) const { return m_data != v.m_data; } + bool operator==(const Vector &v) const { return m_data == v.m_data; } + bool operator!=(const Vector &v) const { return m_data != v.m_data; } /// Tests if all components of the vector are equal to a constant c bool operator==(const T &c) const { - return std::all_of(m_data.begin(), m_data.end(), [&](const T &v) { return v == c; }); + return std::all_of(m_data.begin(), m_data.end(), [&c](const T &v) { return v == c; }); } bool operator!=(const T &c) const { - return std::any_of(m_data.begin(), m_data.end(), [&](const T &v) { return v != c; }); + return std::any_of(m_data.begin(), m_data.end(), [&c](const T &v) { return v != c; }); } - /// The square of the Euclidaen norm of the vector - T NormSquared() const + /// The square of the Euclidean norm of the vector + T NormSquared() const noexcept(noexcept(std::declval() * std::declval())) { - return std::inner_product(m_data.begin(), m_data.end(), m_data.begin(), static_cast(0)); + return std::transform_reduce(m_data.cbegin(), m_data.cend(), T{0}, std::plus<>{}, + [](const T &v) -> T { return v * v; }); } }; +template Vector operator*(const T &p_c, const Vector &p_vector) +{ + return p_vector * p_c; +} + } // end namespace Gambit #endif // GAMBIT_CORE_VECTOR_H From bf1a1068875fefcac72cb2277863124a7933190d Mon Sep 17 00:00:00 2001 From: Theodore Turocy Date: Tue, 9 Dec 2025 13:44:27 +0000 Subject: [PATCH 3/3] Some style improvements to List. --- src/core/list.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/core/list.h b/src/core/list.h index 2f40ff69f..be6c12e45 100644 --- a/src/core/list.h +++ b/src/core/list.h @@ -43,7 +43,6 @@ namespace Gambit { /// Note importantly that the index operator [] is **1-based** - that is, the first /// element of the list is 1, not 0. template class List { -private: std::list m_list; public: @@ -53,10 +52,12 @@ template class List { using size_type = typename std::list::size_type; List() = default; - List(const List &) = default; + List(const List &) = default; + List(List &&) noexcept = default; + List &operator=(List &&) noexcept = default; ~List() = default; - List &operator=(const List &) = default; + List &operator=(const List &) = default; const T &operator[](size_type p_index) const { @@ -109,8 +110,8 @@ template class List { /// Returns a reference to the last element in the list container. const T &back() const { return m_list.back(); } - bool operator==(const List &b) const { return m_list == b.m_list; } - bool operator!=(const List &b) const { return m_list != b.m_list; } + bool operator==(const List &b) const { return m_list == b.m_list; } + bool operator!=(const List &b) const { return m_list != b.m_list; } /// Return a forward iterator starting at the beginning of the list iterator begin() { return m_list.begin(); }