Various syntax updates

Mostly cleaning up issues identified by clang-tidy

But also cleaning up loops, using C++17 syntax when possible

lib/coek/coek/abstract/expr_rule.hpp

@@ -92,7 +92,7 @@ class RangeSet : public IndexSet {
     unsigned int N;
 
    public:
-    RangeSet(unsigned int _N, std::shared_ptr<IndexVariable> i) : N(_N) { index.push_back(i); }
+    RangeSet(unsigned int N_, std::shared_ptr<IndexVariable> i) : N(N_) { index.push_back(i); }
 
     void reset() { i = 0; }
 

lib/coek/coek/api/expression.cpp

@@ -76,7 +76,8 @@ IndexParameter::IndexParameter(const IndexParameter& expr) : repn(expr.repn) {}
 
 IndexParameter& IndexParameter::operator=(const IndexParameter& expr)
 {
-    repn = expr.repn;
+    if (this != &expr)
+        repn = expr.repn;
     return *this;
 }
 

lib/coek/coek/api/objective.cpp

@@ -7,7 +7,7 @@
 
 namespace coek {
 
-Objective::Objective() { repn = CREATE_POINTER(ObjectiveTerm); }
+Objective::Objective() { repn = std::make_shared<ObjectiveTerm>(); }
 
 Objective::Objective(const ObjectiveRepn& _repn) : repn(_repn) {}
 

lib/coek/coek/ast/ast_operators.hpp

@@ -237,17 +237,9 @@ expr_pointer_t times(int lhs, const RHS& rhs)
 inline expr_pointer_t divide_(const expr_pointer_t& lhs, const expr_pointer_t& rhs)
 {
     expr_pointer_t tmp;
-    if (lhs == ZEROCONST)
-        tmp = ZEROCONST;
-    else if (lhs->is_constant() and (safe_pointer_cast<ConstantTerm>(lhs)->value == 0))
+    if ((lhs == ZEROCONST)
+        or (lhs->is_constant() and (safe_pointer_cast<ConstantTerm>(lhs)->value == 0)))
         tmp = ZEROCONST;
-    /* WEH - Not used in practice
-    if (rhs == ONECONST)
-    if (rhs == NEGATIVEONECONST)
-    if (rhs == ZEROCONST)
-    if (rhs->is_constant()) {
-    if (lhs->is_constant() and rhs->is_constant()) {
-    */
     else
         tmp = CREATE_POINTER(DivideTerm, lhs, rhs);
     return tmp;

lib/coek/coek/ast/visitor_simplify.cpp

@@ -23,7 +23,7 @@ class VisitorData {
     bool is_value;
 
     VisitorData(std::map<std::shared_ptr<SubExpressionTerm>, expr_pointer_t>& _subexpr_value)
-        : subexpr_value(_subexpr_value)
+        : subexpr_value(_subexpr_value), last_value(0.0), is_value(true)
     {
     }
 };

lib/coek/coek/autograd/cppad_repn.cpp

@@ -684,16 +684,16 @@ class VisitorData {
     std::map<ParameterRepn, size_t>& parameters;
     std::vector<CppAD::AD<double> >& dynamic_params;
 
-    VisitorData(std::vector<CppAD::AD<double> >& _ADvars,
-                std::unordered_map<VariableRepn, size_t>& _used_variables,
-                std::map<VariableRepn, size_t>& _fixed_variables,
-                std::map<ParameterRepn, size_t>& _parameters,
-                std::vector<CppAD::AD<double> >& _dynamic_params)
-        : ADvars(_ADvars),
-          used_variables(_used_variables),
-          fixed_variables(_fixed_variables),
-          parameters(_parameters),
-          dynamic_params(_dynamic_params)
+    VisitorData(std::vector<CppAD::AD<double> >& ADvars_,
+                std::unordered_map<VariableRepn, size_t>& used_variables_,
+                std::map<VariableRepn, size_t>& fixed_variables_,
+                std::map<ParameterRepn, size_t>& parameters_,
+                std::vector<CppAD::AD<double> >& dynamic_params_)
+        : ADvars(ADvars_),
+          used_variables(used_variables_),
+          fixed_variables(fixed_variables_),
+          parameters(parameters_),
+          dynamic_params(dynamic_params_)
     {
     }
 };

lib/coek/coek/model/writer_nl.cpp

@@ -1169,8 +1169,8 @@ void NLWriter::write_ostream(Model& model, const std::string& fname)
         if (r.size() > 0) {
             ostr << "r\n";
             ctr = 0;
-            for (auto it = r.begin(); it != r.end(); ++it, ++ctr) {
-                switch (*it) {
+            for (auto val : r) {
+                switch (val) {
                     case 0:
                         ostr << "0 ";
                         format(ostr, rval[2 * ctr]);
@@ -1194,17 +1194,21 @@ void NLWriter::write_ostream(Model& model, const std::string& fname)
                         ostr << "4 ";
                         format(ostr, rval[2 * ctr]);
                         break;
+                    default:
+                        // ERROR
+                        break;
                 };
                 ostr << '\n';
+                ++ctr;
             }
         }
 
         //
         // "b" section - bounds on variables
         //
         ostr << "b\n";
-        for (auto it = vars.begin(); it != vars.end(); ++it) {
-            auto var = varobj[*it];
+        for (auto& var_ : vars) {
+            auto var = varobj[var_];
             double lb = var.lower();
             double ub = var.upper();
             if (lb == -COEK_INFINITY) {
@@ -1422,6 +1426,9 @@ void NLWriter::write_fmtlib(Model& model, const std::string& fname)
                 case 4:
                     ostr.print(fmt::format(_fmtstr_r4, rval[2 * ctr]));  // FORMAT
                     break;
+                default:
+                    // ERROR
+                    break;
             };
         }
     }

lib/coek/coek/solvers/gurobi/gurobi.cpp

@@ -518,6 +518,12 @@ std::shared_ptr<SolverResults> GurobiSolver::resolve()
                         = "Error initializing Gurobi: Cannot optimize models with nonlinear terms.";
                     return results;
                     break;
+
+                default:  // Error
+                    results->termination_condition = TerminationCondition::error;
+                    results->error_message = "Error initializing Gurobi: Unexpected error.";
+                    return results;
+                    break;
             };
         }
 
@@ -577,14 +583,16 @@ void GurobiSolver::collect_results(Model& model, std::shared_ptr<SolverResults>&
                 double value = gmodel->get(GRB_DoubleAttr_ObjBound);
                 results->objective_bound = value;
             }
-            catch (GRBException) {
+            catch (GRBException e) {
+                throw;
             }
             if (not results->objective_bound.has_value()) {
                 try {
                     double value = gmodel->get(GRB_DoubleAttr_ObjBoundC);
                     results->objective_bound = value;
                 }
-                catch (GRBException) {
+                catch (GRBException e) {
+                    throw;
                 }
             }
             results->error_message

lib/coek/coek/solvers/solver_repn.cpp

@@ -27,18 +27,18 @@ void SolverCache::find_updated_values()
     vupdates.clear();
     pupdates.clear();
 
-    for (auto it = vcache.begin(); it != vcache.end(); ++it) {
-        auto value = it->first->value->eval();
-        if (fabs(it->second - value) > tolerance) {
-            vupdates.insert(it->first);
-            it->second = value;
+    for (auto& [var, oldval] : vcache) {
+        auto value = var->value->eval();
+        if (fabs(oldval - value) > tolerance) {
+            vupdates.insert(var);
+            oldval = value;
         }
     }
-    for (auto it = pcache.begin(); it != pcache.end(); ++it) {
-        auto value = it->first->eval();
-        if (fabs(it->second - value) > tolerance) {
-            pupdates.insert(it->first);
-            it->second = value;
+    for (auto& [param, oldval] : pcache) {
+        auto value = param->eval();
+        if (fabs(oldval - value) > tolerance) {
+            pupdates.insert(param);
+            oldval = value;
         }
     }
 
@@ -57,35 +57,6 @@ void SolverCache::reset_cache()
     pcache.clear();
 }
 
-/*
-SolverRepn* create_solver(std::string& name, OptionCache& options)
-{
-    if (name == "test")
-        return new TestSolver();
-
-#ifdef WITH_GUROBI
-    if (name == "gurobi") {
-        auto tmp = new GurobiSolver();
-        tmp->set_options(options);
-        return tmp;
-    }
-#endif
-
-    return 0;
-}
-
-NLPSolverRepn* create_nlpsolver(std::string& name, OptionCache& options)
-{
-    if (name == "ipopt") {
-        auto tmp = new IpoptSolver();
-        tmp->set_options(options);
-        return tmp;
-    }
-
-    return 0;
-}
-*/
-
 std::shared_ptr<SolverResults> NLPSolverRepn::resolve(bool reset_nlpmodel)
 {
     if (reset_nlpmodel)
@@ -204,25 +175,25 @@ std::shared_ptr<SolverResults> SolverRepn::solve(CompactModel& _model)
 bool SolverRepn::initial_solve()
 {
     if (initial) {
-        for (auto it = vconstvals.begin(); it != vconstvals.end(); ++it)
-            vcache[it->first] = it->first->eval();
-        for (auto it = pconstvals.begin(); it != pconstvals.end(); ++it)
-            pcache[it->first] = it->first->eval();
-
-        for (auto it = vlinvals.begin(); it != vlinvals.end(); ++it)
-            vcache[it->first] = it->first->eval();
-        for (auto it = plinvals.begin(); it != plinvals.end(); ++it)
-            pcache[it->first] = it->first->eval();
-
-        for (auto it = vquadvals.begin(); it != vquadvals.end(); ++it)
-            vcache[it->first] = it->first->eval();
-        for (auto it = pquadvals.begin(); it != pquadvals.end(); ++it)
-            pcache[it->first] = it->first->eval();
-
-        for (auto it = vnonlvals.begin(); it != vnonlvals.end(); ++it)
-            vcache[it->first] = it->first->eval();
-        for (auto it = pnonlvals.begin(); it != pnonlvals.end(); ++it)
-            pcache[it->first] = it->first->eval();
+        for (auto& [vconst, _] : vconstvals)
+            vcache[vconst] = vconst->eval();
+        for (auto& [pconst, _] : pconstvals)
+            pcache[pconst] = pconst->eval();
+
+        for (auto& [vlin, _] : vlinvals)
+            vcache[vlin] = vlin->eval();
+        for (auto& [plin, _] : plinvals)
+            pcache[plin] = plin->eval();
+
+        for (auto& [vquad, _] : vquadvals)
+            vcache[vquad] = vquad->eval();
+        for (auto& [pquad, _] : pquadvals)
+            pcache[pquad] = pquad->eval();
+
+        for (auto& [vnonl, _] : vnonlvals)
+            vcache[vnonl] = vnonl->eval();
+        for (auto& [pnonl, _] : pnonlvals)
+            pcache[pnonl] = pnonl->eval();
 
         vupdates.clear();
         pupdates.clear();
@@ -243,35 +214,33 @@ void SolverRepn::find_updated_coefs()
 
     for (auto it = vupdates.begin(); it != vupdates.end(); ++it) {
         try {
-            std::set<size_t>& expr = vconstvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(std::tuple<size_t, size_t, size_t>(*jt, 0, 0));
+            // std::set<size_t>& expr = vconstvals[*it];
+            for (size_t j : vconstvals[*it])
+                updated_coefs.insert({j, 0, 0});
         }
         catch (...) {
             // TODO
         }
         try {
-            std::set<std::tuple<size_t, size_t>>& expr = vlinvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(
-                    std::tuple<size_t, size_t, size_t>(std::get<0>(*jt), 1, std::get<1>(*jt)));
+            // std::set<std::tuple<size_t, size_t>>& expr = vlinvals[*it];
+            for (auto& [i, j] : vlinvals[*it])
+                updated_coefs.insert({i, 1, j});
         }
         catch (...) {
             // TODO
         }
         try {
-            std::set<std::tuple<size_t, size_t>>& expr = vquadvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(
-                    std::tuple<size_t, size_t, size_t>(std::get<0>(*jt), 2, std::get<1>(*jt)));
+            // std::set<std::tuple<size_t, size_t>>& expr = vquadvals[*it];
+            for (auto& [i, j] : vquadvals[*it])
+                updated_coefs.insert({i, 2, j});
         }
         catch (...) {
             // TODO
         }
         try {
-            std::set<size_t>& expr = vnonlvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(std::tuple<size_t, size_t, size_t>(*jt, 3, 0));
+            // std::set<size_t>& expr = vnonlvals[*it];
+            for (size_t j : vnonlvals[*it])
+                updated_coefs.insert({j, 3, 0});
         }
         catch (...) {
             // TODO
@@ -280,35 +249,33 @@ void SolverRepn::find_updated_coefs()
 
     for (auto it = pupdates.begin(); it != pupdates.end(); ++it) {
         try {
-            std::set<size_t>& expr = pconstvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(std::tuple<size_t, size_t, size_t>(*jt, 0, 0));
+            // std::set<size_t>& expr = pconstvals[*it];
+            for (size_t j : pconstvals[*it])
+                updated_coefs.insert({j, 0, 0});
         }
         catch (...) {
             // TODO
         }
         try {
-            std::set<std::tuple<size_t, size_t>>& expr = plinvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(
-                    std::tuple<size_t, size_t, size_t>(std::get<0>(*jt), 1, std::get<1>(*jt)));
+            // std::set<std::tuple<size_t, size_t>>& expr = plinvals[*it];
+            for (auto& [i, j] : plinvals[*it])
+                updated_coefs.insert({i, 1, j});
         }
         catch (...) {
             // TODO
         }
         try {
-            std::set<std::tuple<size_t, size_t>>& expr = pquadvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(
-                    std::tuple<size_t, size_t, size_t>(std::get<0>(*jt), 2, std::get<1>(*jt)));
+            // std::set<std::tuple<size_t, size_t>>& expr = pquadvals[*it];
+            for (auto& [i, j] : pquadvals[*it])
+                updated_coefs.insert({i, 2, j});
         }
         catch (...) {
             // TODO
         }
         try {
-            std::set<size_t>& expr = pnonlvals[*it];
-            for (auto jt = expr.begin(); jt != expr.end(); ++jt)
-                updated_coefs.insert(std::tuple<size_t, size_t, size_t>(*jt, 3, 0));
+            // std::set<size_t>& expr = pnonlvals[*it];
+            for (size_t j : pnonlvals[*it])
+                updated_coefs.insert({j, 3, 0});
         }
         catch (...) {
             // TODO

lib/coek/coek/util/index_vector.hpp

@@ -31,6 +31,9 @@ class IndexVector {
 
     IndexVector& operator=(const IndexVector& other)
     {
+        if (this == &other)
+            return *this;
+
         len = other.len;
         data = other.data;
         return *this;
@@ -129,8 +132,9 @@ struct hash<coek::IndexVector> {
 
 inline std::ostream& operator<<(std::ostream& ostr, const coek::IndexVector& vec)
 {
-    for (size_t i = 0; i < vec.size(); i++)
+    for (size_t i = 0; i < vec.size(); i++) {
         ostr << vec[i] << " ";
+    }
     return ostr;
 }