commit 9665e099022bd06e53b0779550e9aebded7f274d
author Sameer Agarwal <sameeragarwal@google.com> Mon Apr 18 06:00:58 2016 -0700
committer Keir Mierle <mierle@gmail.com> Tue Apr 19 00:08:06 2016 +0000
tree 894e489f0cbae5c77f8c0a3b26df7dc633aec77e
parent 620ca9d0668cd4a00402264fddca3cf6bd2e7265

Fix step norm evaluation in LineSearchMinimizer

TrustRegionMinimizer evaluates the size of the step
taken in the ambient space, where as the LineSearchMinimizer
was using the norm in the tangent space. This change fixes
this discrepancy.

Change-Id: I9fef64cbb5622c9769c0413003cfb1dc6e89cfa3

internal/ceres/line_search_minimizer.cc

diff --git a/internal/ceres/line_search_minimizer.cc b/internal/ceres/line_search_minimizer.cc
index f424dd8..be85ceb 100644
--- a/internal/ceres/line_search_minimizer.cc
+++ b/internal/ceres/line_search_minimizer.cc
@@ -341,10 +341,12 @@
           "as the step was valid when it was selected by the line search.";
       LOG_IF(WARNING, is_not_silent) << "Terminating: " << summary->message;
       break;
-    } else if (!Evaluate(evaluator,
-                         x_plus_delta,
-                         &current_state,
-                         &summary->message)) {
+    }
+
+    if (!Evaluate(evaluator,
+                  x_plus_delta,
+                  &current_state,
+                  &summary->message)) {
       summary->termination_type = FAILURE;
       summary->message =
           "Step failed to evaluate. This should not happen as the step was "
@@ -352,15 +354,17 @@
           summary->message;
       LOG_IF(WARNING, is_not_silent) << "Terminating: " << summary->message;
       break;
-    } else {
-      x = x_plus_delta;
     }
 
+    // Compute the norm of the step in the ambient space.
+    iteration_summary.step_norm = (x_plus_delta - x).norm();
+    x = x_plus_delta;
+
     iteration_summary.gradient_max_norm = current_state.gradient_max_norm;
     iteration_summary.gradient_norm = sqrt(current_state.gradient_squared_norm);
     iteration_summary.cost_change = previous_state.cost - current_state.cost;
     iteration_summary.cost = current_state.cost + summary->fixed_cost;
-    iteration_summary.step_norm = delta.norm();
+
     iteration_summary.step_is_valid = true;
     iteration_summary.step_is_successful = true;
     iteration_summary.step_size =  current_state.step_size;

internal/ceres/trust_region_minimizer.cc

diff --git a/internal/ceres/trust_region_minimizer.cc b/internal/ceres/trust_region_minimizer.cc
index 35c7c41..d809906 100644
--- a/internal/ceres/trust_region_minimizer.cc
+++ b/internal/ceres/trust_region_minimizer.cc
@@ -665,6 +665,7 @@
 
 // Solver::Options::parameter_tolerance based convergence check.
 bool TrustRegionMinimizer::ParameterToleranceReached() {
+  // Compute the norm of the step in the ambient space.
   iteration_summary_.step_norm = (x_ - candidate_x_).norm();
   const double step_size_tolerance =
       options_.parameter_tolerance * (x_norm_ + options_.parameter_tolerance);