Refactor constraint handling: replace shared pointers with references and update related methods for improved memory management and consistency.

Author: riccardodebenedictis

include/linspire.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
 #include "var.hpp"
-#include <set>
 #include <unordered_map>
 
 namespace linspire
@@ -124,10 +123,10 @@ namespace linspire
      * @param lhs The left-hand side linear expression of the constraint.
      * @param rhs The right-hand side linear expression of the constraint.
      * @param strict A boolean indicating whether the constraint is strict (default: false).
-     * @param reason An optional shared pointer to a constraint object that serves as the reason for this constraint.
+     * @param reason An optional constraint that serves as the reason for adding this new constraint.
      * @return true if the constraint was successfully added; false if it leads to inconsistency.
      */
-    [[nodiscard]] bool new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, std::shared_ptr<constraint> reason = nullptr) noexcept;
+    [[nodiscard]] bool new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, const std::optional<std::reference_wrapper<constraint>> &reason = std::nullopt) noexcept;
     /**
      * @brief Adds a new equality constraint to the solver.
      *
@@ -136,10 +135,10 @@ namespace linspire
      *
      * @param lhs The left-hand side linear expression of the equality constraint.
      * @param rhs The right-hand side linear expression of the equality constraint.
-     * @param reason An optional shared pointer to a constraint object that serves as the reason for this equality constraint.
+     * @param reason An optional constraint that serves as the reason for adding this new equality constraint.
      * @return true if the equality constraint was successfully added; false if it leads to inconsistency.
      */
-    [[nodiscard]] bool new_eq(const utils::lin &lhs, const utils::lin &rhs, std::shared_ptr<constraint> reason = nullptr) noexcept;
+    [[nodiscard]] bool new_eq(const utils::lin &lhs, const utils::lin &rhs, const std::optional<std::reference_wrapper<constraint>> &reason = std::nullopt) noexcept;
     /**
      * @brief Adds a new greater-than or greater-than-or-equal-to constraint to the solver.
      *
@@ -150,20 +149,20 @@ namespace linspire
      * @param lhs The left-hand side linear expression of the constraint.
      * @param rhs The right-hand side linear expression of the constraint.
      * @param strict A boolean indicating whether the constraint is strict (default: false).
-     * @param reason An optional shared pointer to a constraint object that serves as the reason for this constraint.
+     * @param reason An optional constraint that serves as the reason for adding this new constraint.
      * @return true if the constraint was successfully added; false if it leads to inconsistency.
      */
-    [[nodiscard]] bool new_gt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, std::shared_ptr<constraint> reason = nullptr) noexcept { return new_lt(rhs, lhs, strict, reason); }
+    [[nodiscard]] bool new_gt(const utils::lin &lhs, const utils::lin &rhs, bool strict = false, const std::optional<std::reference_wrapper<constraint>> &reason = std::nullopt) noexcept { return new_lt(rhs, lhs, strict, reason); }
 
     /**
      * @brief Retracts a previously added constraint from the solver.
      *
      * This function removes a previously added constraint from the solver. It updates the
      * internal state of the solver to reflect the removal of the constraint.
      *
-     * @param c A shared pointer to the constraint object to be retracted.
+     * @param c The constraint to be retracted.
      */
-    void retract(const std::shared_ptr<constraint> c) noexcept;
+    void retract(constraint &c) noexcept;
 
     /**
      * @brief Checks the consistency of the current set of constraints.
@@ -184,9 +183,9 @@ namespace linspire
      * The conflict explanation consists of a set of constraints that led to an inconsistency
      * in the solver.
      *
-     * @return A constant reference to a vector of shared pointers to constraint objects representing the last conflict explanation.
+     * @return A constant reference to the vector of constraints representing the last conflict explanation.
      */
-    [[nodiscard]] const std::vector<std::shared_ptr<const constraint>> &get_conflict() const noexcept { return cnfl; }
+    [[nodiscard]] const std::vector<std::reference_wrapper<constraint>> &get_conflict() const noexcept { return cnfl; }
 
     /**
      * @brief Checks if two linear expressions can be made equal.
@@ -204,21 +203,21 @@ namespace linspire
     /**
      * @brief Adds a listener to the solver.
      *
-     * This function registers a listener that will be notified of variable value changes
+     * This function registers a listener that will be notified of changes to variable values
      * within the solver. The listener must implement the `listener` interface.
      *
-     * @param l A shared pointer to the listener to be added.
+     * @param l A reference to the listener to be added.
      */
-    void add_listener(std::shared_ptr<listener> l) noexcept;
+    void add_listener(listener &l) noexcept;
     /**
      * @brief Removes a listener from the solver.
      *
-     * This function unregisters a previously added listener from the solver. The listener
-     * will no longer receive notifications of variable value changes.
+     * This function unregisters a previously added listener from the solver. The listener will
+     * no longer receive notifications of variable value changes.
      *
-     * @param l A shared pointer to the listener to be removed.
+     * @param l A reference to the listener to be removed.
      */
-    void remove_listener(std::shared_ptr<listener> l) noexcept;
+    void remove_listener(listener &l) noexcept;
 #endif
 
     friend std::string to_string(const solver &s) noexcept;
@@ -227,8 +226,8 @@ namespace linspire
   private:
     [[nodiscard]] bool is_basic(const utils::var v) const noexcept { return tableau.count(v); }
 
-    [[nodiscard]] bool set_lb(const utils::var x_i, const utils::inf_rational &v, std::shared_ptr<constraint> reason = nullptr) noexcept;
-    [[nodiscard]] bool set_ub(const utils::var x_i, const utils::inf_rational &v, std::shared_ptr<constraint> reason = nullptr) noexcept;
+    [[nodiscard]] bool set_lb(const utils::var x_i, const utils::inf_rational &v, const std::optional<std::reference_wrapper<constraint>> &reason = std::nullopt) noexcept;
+    [[nodiscard]] bool set_ub(const utils::var x_i, const utils::inf_rational &v, const std::optional<std::reference_wrapper<constraint>> &reason = std::nullopt) noexcept;
 
     void update(const utils::var x_i, const utils::inf_rational &v) noexcept;
 
@@ -238,14 +237,14 @@ namespace linspire
 
     void new_row(const utils::var x, utils::lin &&l) noexcept;
 
-    std::vector<var> vars;                               // index is the variable id
-    std::unordered_map<std::string, utils::var> exprs;   // the expressions (string to numeric variable) for which already exist slack variables..
-    std::map<utils::var, utils::lin> tableau;            // basic variable -> expression
-    std::vector<std::set<utils::var>> t_watches;         // for each variable `v`, a set of tableau rows watching `v`..
-    std::vector<std::shared_ptr<const constraint>> cnfl; // the last conflict explanation..
+    std::vector<var> vars;                                // index is the variable id
+    std::unordered_map<std::string, utils::var> exprs;    // the expressions (string to numeric variable) for which already exist slack variables..
+    std::map<utils::var, utils::lin> tableau;             // basic variable -> expression
+    std::vector<std::set<utils::var>> t_watches;          // for each variable `v`, a set of tableau rows watching `v`..
+    std::vector<std::reference_wrapper<constraint>> cnfl; // the last conflict explanation..
 #ifdef LINSPIRE_ENABLE_LISTENERS
     std::unordered_map<utils::var, std::set<listener *>> listening; // for each variable, the listeners listening to it..
-    std::set<std::shared_ptr<listener>> listeners;                  // the collection of listeners..
+    std::set<listener *> listeners;                                 // the collection of listeners..
 #endif
   };
 

include/var.hpp

@@ -4,7 +4,7 @@
 #include "inf_rational.hpp"
 #include "json.hpp"
 #include <set>
-#include <memory>
+#include <optional>
 
 namespace linspire
 {
@@ -25,14 +25,14 @@ namespace linspire
     friend json::json to_json(const var &x) noexcept;
 
   private:
-    void set_lb(const utils::inf_rational &v, std::shared_ptr<const constraint> reason = nullptr) noexcept;
-    void unset_lb(const utils::inf_rational &v, std::shared_ptr<const constraint> reason) noexcept;
-    void set_ub(const utils::inf_rational &v, std::shared_ptr<const constraint> reason = nullptr) noexcept;
-    void unset_ub(const utils::inf_rational &v, std::shared_ptr<const constraint> reason) noexcept;
+    void set_lb(const utils::inf_rational &v, std::optional<constraint> reason = std::nullopt) noexcept;
+    void unset_lb(const utils::inf_rational &v, constraint &reason) noexcept;
+    void set_ub(const utils::inf_rational &v, std::optional<constraint> reason = std::nullopt) noexcept;
+    void unset_ub(const utils::inf_rational &v, constraint &reason) noexcept;
 
   private:
-    std::map<utils::inf_rational, std::set<std::shared_ptr<const constraint>>> lbs, ubs; // the lower and upper bounds with their reasons..
-    utils::inf_rational val;                                                             // the current value of this variable..
+    std::map<utils::inf_rational, std::set<constraint *>> lbs, ubs; // the lower and upper bounds with their reasons..
+    utils::inf_rational val;                                        // the current value of this variable..
   };
 
   [[nodiscard]] std::string to_string(const var &x) noexcept;

src/linspire.cpp

@@ -41,7 +41,7 @@ namespace linspire
     utils::inf_rational solver::ub(const utils::var x) const noexcept { return vars[x].get_ub(); }
     utils::inf_rational solver::val(const utils::var x) const noexcept { return vars[x].val; }
 
-    bool solver::new_eq(const utils::lin &lhs, const utils::lin &rhs, std::shared_ptr<constraint> reason) noexcept
+    bool solver::new_eq(const utils::lin &lhs, const utils::lin &rhs, const std::optional<std::reference_wrapper<constraint>> &reason) noexcept
     {
         LOG_TRACE(utils::to_string(lhs) + " == " + utils::to_string(rhs));
         utils::lin expr = lhs - rhs;
@@ -79,7 +79,7 @@ namespace linspire
         }
     }
 
-    bool solver::new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict, std::shared_ptr<constraint> reason) noexcept
+    bool solver::new_lt(const utils::lin &lhs, const utils::lin &rhs, bool strict, const std::optional<std::reference_wrapper<constraint>> &reason) noexcept
     {
         LOG_TRACE(utils::to_string(lhs) + (strict ? " < " : " <= ") + utils::to_string(rhs));
         utils::lin expr = lhs - rhs;
@@ -119,14 +119,14 @@ namespace linspire
         }
     }
 
-    void solver::retract(const std::shared_ptr<constraint> c) noexcept
+    void solver::retract(constraint &c) noexcept
     {
-        for (const auto &[x, lb] : c->lbs)
+        for (const auto &[x, lb] : c.lbs)
             vars[x].unset_lb(lb, c);
-        c->lbs.clear();
-        for (const auto &[x, ub] : c->ubs)
+        c.lbs.clear();
+        for (const auto &[x, ub] : c.ubs)
             vars[x].unset_ub(ub, c);
-        c->ubs.clear();
+        c.ubs.clear();
     }
 
     bool solver::check() noexcept
@@ -152,12 +152,12 @@ namespace linspire
                     for (const auto &[v, c] : l.vars)
                         if (is_positive(c)) // we use the most restrictive upper bound of v
                             for (const auto &w : vars.at(v).ubs.begin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                         else if (is_negative(c)) // we use the most restrictive lower bound of v
                             for (const auto &w : vars.at(v).lbs.rbegin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                     for (const auto &w : vars.at(x_i).lbs.rbegin()->second) // we use the most restrictive lower bound of x_i
-                        cnfl.push_back(w);
+                        cnfl.push_back(*w);
                     return false;
                 }
             }
@@ -173,12 +173,12 @@ namespace linspire
                     for (const auto &[v, c] : l.vars)
                         if (is_positive(c)) // we use the most restrictive lower bound of v
                             for (const auto &w : vars.at(v).lbs.rbegin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                         else if (is_negative(c)) // we use the most restrictive upper bound of v
                             for (const auto &w : vars.at(v).ubs.begin()->second)
-                                cnfl.push_back(w);
+                                cnfl.push_back(*w);
                     for (const auto &w : vars.at(x_i).ubs.begin()->second) // we use the most restrictive upper bound of x_i
-                        cnfl.push_back(w);
+                        cnfl.push_back(*w);
                     return false;
                 }
             }
@@ -187,7 +187,7 @@ namespace linspire
 
     bool solver::match(const utils::lin &l0, const utils::lin &l1) const noexcept { return lb(l0) <= ub(l1) && ub(l0) >= lb(l1); }
 
-    bool solver::set_lb(const utils::var x, const utils::inf_rational &v, std::shared_ptr<constraint> reason) noexcept
+    bool solver::set_lb(const utils::var x, const utils::inf_rational &v, const std::optional<std::reference_wrapper<constraint>> &reason) noexcept
     {
         assert(x < vars.size());
         assert(v > utils::rational::negative_infinite);
@@ -196,30 +196,30 @@ namespace linspire
         { // inconsistent bound..
             cnfl.clear();
             if (reason)
-                cnfl.push_back(reason);
+                cnfl.push_back(reason.value());
             for (const auto &w : vars.at(x).ubs.begin()->second) // we use the most restrictive upper bound of x
-                cnfl.push_back(w);
+                cnfl.push_back(*w);
             return false;
         }
         if (reason)
         { // we have a reason for this bound..
-            if (auto it = reason->lbs.find(x); it != reason->lbs.end())
+            if (auto it = reason->get().lbs.find(x); it != reason->get().lbs.end())
             { // we already have a lower bound for this variable in the reason..
                 if (it->second < v)
                 { // we update the lower bound only if the new one is more restrictive..
-                    vars.at(x).unset_lb(it->second, reason);
+                    vars.at(x).unset_lb(it->second, reason->get());
                     it->second = v;
                 }
             }
             else
-                reason->lbs.emplace(x, v);
+                reason->get().lbs.emplace(x, v);
         }
         vars.at(x).set_lb(v, reason);
         if (val(x) < v && !is_basic(x))
             update(x, v);
         return true;
     }
-    bool solver::set_ub(const utils::var x, const utils::inf_rational &v, std::shared_ptr<constraint> reason) noexcept
+    bool solver::set_ub(const utils::var x, const utils::inf_rational &v, const std::optional<std::reference_wrapper<constraint>> &reason) noexcept
     {
         assert(x < vars.size());
         assert(v < utils::rational::positive_infinite);
@@ -228,23 +228,23 @@ namespace linspire
         { // inconsistent bound..
             cnfl.clear();
             if (reason)
-                cnfl.push_back(reason);
+                cnfl.push_back(reason.value());
             for (const auto &w : vars.at(x).lbs.rbegin()->second) // we use the most restrictive lower bound of x
-                cnfl.push_back(w);
+                cnfl.push_back(*w);
             return false;
         }
         if (reason)
         { // we have a reason for this bound..
-            if (auto it = reason->ubs.find(x); it != reason->ubs.end())
+            if (auto it = reason->get().ubs.find(x); it != reason->get().ubs.end())
             { // we already have an upper bound for this variable in the reason..
                 if (it->second > v)
                 { // we update the upper bound only if the new one is more restrictive..
-                    vars.at(x).unset_ub(it->second, reason);
+                    vars.at(x).unset_ub(it->second, reason->get());
                     it->second = v;
                 }
             }
             else
-                reason->ubs.emplace(x, v);
+                reason->get().ubs.emplace(x, v);
         }
         vars.at(x).set_ub(v, reason);
         if (val(x) > v && !is_basic(x))
@@ -367,8 +367,8 @@ namespace linspire
     }
 
 #ifdef LINSPIRE_ENABLE_LISTENERS
-    void solver::add_listener(std::shared_ptr<listener> l) noexcept { listeners.insert(l); }
-    void solver::remove_listener(std::shared_ptr<listener> l) noexcept { listeners.erase(l); }
+    void solver::add_listener(listener &l) noexcept { listeners.insert(&l); }
+    void solver::remove_listener(listener &l) noexcept { listeners.erase(&l); }
 #endif
 
     std::string to_string(const solver &s) noexcept