ls clenup

polyfem · Oct 18, 2023 · 17c809a · 17c809a
1 parent 2586b62
commit 17c809a
Show file tree

Hide file tree

Showing 8 changed files with 32 additions and 25 deletions.
diff --git a/src/polysolve/nonlinear/Solver.cpp b/src/polysolve/nonlinear/Solver.cpp
@@ -402,7 +402,7 @@ namespace polysolve::nonlinear
 
         if (m_line_search)
         {
-            solver_info["line_search_iterations"] = m_line_search->iterations;
+            solver_info["line_search_iterations"] = m_line_search->iterations();
 
             solver_info["time_checking_for_nan_inf"] =
                 m_line_search->checking_for_nan_inf_time / per_iteration;

diff --git a/src/polysolve/nonlinear/line_search/Backtracking.cpp b/src/polysolve/nonlinear/line_search/Backtracking.cpp
@@ -31,8 +31,6 @@ namespace polysolve::nonlinear::line_search
         bool is_step_valid = false;
         while (step_size > current_min_step_size() && cur_iter < current_max_step_size_iter())
         {
-            iterations++;
-
             TVector new_x = x + step_size * delta_x;
 
             try
@@ -44,7 +42,7 @@ namespace polysolve::nonlinear::line_search
             {
                 m_logger.warn("Failed to take step due to \"{}\", reduce step size...", e.what());
 
-                step_size /= 2.0;
+                step_size *= step_ratio;
                 this->cur_iter++;
                 continue;
             }
@@ -63,7 +61,7 @@ namespace polysolve::nonlinear::line_search
 
             if (!std::isfinite(cur_energy) || cur_energy >= old_energy || !is_step_valid)
             {
-                step_size /= 2.0;
+                step_size *= step_ratio;
                 // max_step_size should return a collision free step
                 // assert(objFunc.is_step_collision_free(x, new_x));
             }

diff --git a/src/polysolve/nonlinear/line_search/Backtracking.hpp b/src/polysolve/nonlinear/line_search/Backtracking.hpp
@@ -15,7 +15,7 @@ namespace polysolve::nonlinear::line_search
 
         virtual std::string name() override { return "Backtracking"; }
 
-    protected:
+    public:
         double compute_descent_step_size(
             const TVector &x,
             const TVector &delta_x,

diff --git a/src/polysolve/nonlinear/line_search/CppOptArmijo.cpp b/src/polysolve/nonlinear/line_search/CppOptArmijo.cpp
@@ -5,21 +5,21 @@
 namespace polysolve::nonlinear::line_search
 {
     CppOptArmijo::CppOptArmijo(const json &params, spdlog::logger &logger)
-        : Superclass(params, logger)
+        : Superclass(params, logger), after_check(params, logger)
     {
     }
 
     double CppOptArmijo::compute_descent_step_size(
         const TVector &x,
         const TVector &delta_x,
         Problem &objFunc,
-        const bool,
-        const double,
+        const bool use_grad_norm,
+        const double old_energy,
         const double starting_step_size)
     {
-        const double tmp = cppoptlib::Armijo<Problem, 1>::linesearch(x, delta_x, objFunc, starting_step_size);
-        assert(tmp <= starting_step_size);
-        return tmp;
+        double step_size = cppoptlib::Armijo<Problem, 1>::linesearch(x, delta_x, objFunc, starting_step_size);
+        // this ensures no collisions and decrease in energy
+        return after_check.compute_descent_step_size(x, delta_x, objFunc, use_grad_norm, old_energy, step_size);
     }
 
 } // namespace polysolve::nonlinear::line_search
diff --git a/src/polysolve/nonlinear/line_search/CppOptArmijo.hpp b/src/polysolve/nonlinear/line_search/CppOptArmijo.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include "LineSearch.hpp"
+#include "Backtracking.hpp"
 
 namespace polysolve::nonlinear::line_search
 {
@@ -20,8 +21,11 @@ namespace polysolve::nonlinear::line_search
             const TVector &x,
             const TVector &delta_x,
             Problem &objFunc,
-            const bool,
-            const double,
+            const bool use_grad_norm,
+            const double old_energy,
             const double starting_step_size) override;
+
+    private:
+        Backtracking after_check;
     };
 } // namespace polysolve::nonlinear::line_search
diff --git a/src/polysolve/nonlinear/line_search/LineSearch.hpp b/src/polysolve/nonlinear/line_search/LineSearch.hpp
@@ -31,7 +31,6 @@ namespace polysolve::nonlinear::line_search
 
         void reset_times()
         {
-            iterations = 0;
             checking_for_nan_inf_time = 0;
             broad_phase_ccd_time = 0;
             ccd_time = 0;
@@ -54,7 +53,6 @@ namespace polysolve::nonlinear::line_search
             return is_final_strategy ? max_step_size_iter_final : max_step_size_iter;
         }
 
-        int iterations; ///< total number of backtracking iterations done
         double checking_for_nan_inf_time;
         double broad_phase_ccd_time;
         double ccd_time;
@@ -65,6 +63,8 @@ namespace polysolve::nonlinear::line_search
 
         virtual std::string name() = 0;
 
+        inline int iterations() const { return cur_iter; }
+
     private:
         double min_step_size;
         int max_step_size_iter;
@@ -73,11 +73,12 @@ namespace polysolve::nonlinear::line_search
 
         bool is_final_strategy;
 
+        double default_init_step_size;
+
     protected:
-        int cur_iter = 0;
+        int cur_iter;
         spdlog::logger &m_logger;
 
-        double default_init_step_size;
         double step_ratio;
 
         virtual double compute_descent_step_size(

diff --git a/src/polysolve/nonlinear/line_search/MoreThuente.cpp b/src/polysolve/nonlinear/line_search/MoreThuente.cpp
@@ -5,20 +5,20 @@
 namespace polysolve::nonlinear::line_search
 {
     MoreThuente::MoreThuente(const json &params, spdlog::logger &logger)
-        : Superclass(params, logger)
+        : Superclass(params, logger), after_check(params, logger)
     {
     }
 
     double MoreThuente::compute_descent_step_size(
         const TVector &x,
         const TVector &delta_x,
         Problem &objFunc,
-        const bool,
-        const double,
+        const bool use_grad_norm,
+        const double old_energy,
         const double starting_step_size)
     {
         const double tmp = cppoptlib::MoreThuente<Problem, 1>::linesearch(x, delta_x, objFunc, starting_step_size);
-
-        return std::min(tmp, starting_step_size);
+        // this ensures no collisions and decrease in energy
+        return after_check.compute_descent_step_size(x, delta_x, objFunc, use_grad_norm, old_energy, std::min(tmp, starting_step_size));
     }
 } // namespace polysolve::nonlinear::line_search
diff --git a/src/polysolve/nonlinear/line_search/MoreThuente.hpp b/src/polysolve/nonlinear/line_search/MoreThuente.hpp
@@ -1,6 +1,7 @@
 #pragma once
 
 #include "LineSearch.hpp"
+#include "Backtracking.hpp"
 
 namespace polysolve::nonlinear::line_search
 {
@@ -20,8 +21,11 @@ namespace polysolve::nonlinear::line_search
             const TVector &x,
             const TVector &delta_x,
             Problem &objFunc,
-            const bool,
-            const double,
+            const bool use_grad_norm,
+            const double old_energy,
             const double starting_step_size) override;
+
+    private:
+        Backtracking after_check;
     };
 } // namespace polysolve::nonlinear::line_search