commit a8d9ca8f4ed7137e180cf99f38825f4756174c58
author Sameer Agarwal <sameeragarwal@google.com> Thu Sep 05 22:08:57 2013 -0700
committer Keir Mierle <mierle@gmail.com> Sat Sep 07 16:45:22 2013 +0000
tree 9706993a635ba028a9ab4f13bb687207f4dc1223
parent 90bbf0fb221de94e33ffd1e655baad81f07f9ee4

Fix a bug in TrustRegionMinimizer.

LocalParameterization::Plus is allowed to return false when for some
reason the operation cannot be performed. Uptil now, this would
cause the TrustRegionMinimizer to terminate with a numerical failure.

This is not correct behaviour. Just like CostFunction::Evaluate,
returning false from LocalParameterization::Plus should result
in the current step to be rejected rather than the entire optimization
coming to a halt.

LineSearchMinimizer has also been updated to use the same pattern.

Thanks to Michael Vitus for reporting this.

Change-Id: I1351966bc6db3bf6cd46387b78d4e52a9df51696

internal/ceres/line_search_minimizer.cc

diff --git a/internal/ceres/line_search_minimizer.cc b/internal/ceres/line_search_minimizer.cc
index 2cc89fa..5533e20 100644
--- a/internal/ceres/line_search_minimizer.cc
+++ b/internal/ceres/line_search_minimizer.cc
@@ -69,6 +69,9 @@
 // use.
 const double kEpsilon = 1e-12;
 
+// TODO(sameeragarwal): I think there is a small bug here, in that if
+// the evaluation fails, then the state can contain garbage. Look at
+// this more carefully.
 bool Evaluate(Evaluator* evaluator,
               const Vector& x,
               LineSearchMinimizer::State* state) {
@@ -310,9 +313,10 @@
         WallTimeInSeconds() - iteration_start_time;
 
     // TODO(sameeragarwal): Collect stats.
-    if (!evaluator->Plus(x.data(), delta.data(), x_plus_delta.data()) ||
-        !Evaluate(evaluator, x_plus_delta, &current_state)) {
-      LOG(WARNING) << "Evaluation failed.";
+    if (!evaluator->Plus(x.data(), delta.data(), x_plus_delta.data())) {
+      LOG(WARNING) << "x_plus_delta = Plus(x, delta) failed. ";
+    } else if (!Evaluate(evaluator, x_plus_delta, &current_state)) {
+      LOG(WARNING) << "Step failed to evaluate. ";
     } else {
       x = x_plus_delta;
     }