commit b58a877634c254a45c49eee56f34846163112746
author Sameer Agarwal <sameeragarwal@google.com> Fri Nov 14 10:56:19 2014 -0800
committer Sameer Agarwal <sameeragarwal@google.com> Fri Nov 14 11:18:25 2014 -0800
tree 6147661c3eaf73f337795328f5a257d82f4e8231
parent 993622e915b1d954db95d63f7fa0fa9ee746e594

Better options checking for TrustRegionMinimizer.

Since the trust region minimizer can use the line search
minimizer when it is solving a box constrained problem,
ensure that the line search options are valid.

Also some minor spacing fixes in the line search code.

Change-Id: Ife04204855cfac389cf980f0a79155d4accc8662

internal/ceres/line_search.cc

diff --git a/internal/ceres/line_search.cc b/internal/ceres/line_search.cc
index 7ff1164..0d645ac 100644
--- a/internal/ceres/line_search.cc
+++ b/internal/ceres/line_search.cc
@@ -125,14 +125,12 @@
 
   if (g == NULL) {
     return (evaluator_->Evaluate(evaluation_point_.data(),
-                                  f, NULL, NULL, NULL) &&
+                                 f, NULL, NULL, NULL) &&
             IsFinite(*f));
   }
 
   if (!evaluator_->Evaluate(evaluation_point_.data(),
-                            f,
-                            NULL,
-                            gradient_.data(), NULL)) {
+                            f, NULL, gradient_.data(), NULL)) {
     return false;
   }
 

internal/ceres/solver.cc

diff --git a/internal/ceres/solver.cc b/internal/ceres/solver.cc
index 3512e15..a1f347f 100644
--- a/internal/ceres/solver.cc
+++ b/internal/ceres/solver.cc
@@ -446,11 +446,16 @@
     return false;
   }
 
-  if (minimizer_type == TRUST_REGION) {
-    return TrustRegionOptionsAreValid(*this, error);
+  if (minimizer_type == TRUST_REGION &&
+      !TrustRegionOptionsAreValid(*this, error)) {
+    return false;
   }
 
-  CHECK_EQ(minimizer_type, LINE_SEARCH);
+  // We do not know if the problem is bounds constrained or not, if it
+  // is then the trust region solver will also use the line search
+  // solver to do a projection onto the box constraints, so make sure
+  // that the line search options are checked independent of what
+  // minimizer algorithm is being used.
   return LineSearchOptionsAreValid(*this, error);
 }
 

internal/ceres/trust_region_minimizer.cc

diff --git a/internal/ceres/trust_region_minimizer.cc b/internal/ceres/trust_region_minimizer.cc
index c871ae2..7352ac2 100644
--- a/internal/ceres/trust_region_minimizer.cc
+++ b/internal/ceres/trust_region_minimizer.cc
@@ -86,11 +86,10 @@
   line_search_options.function = &line_search_function;
 
   string message;
-  scoped_ptr<LineSearch>
-      line_search(CHECK_NOTNULL(
-                      LineSearch::Create(ceres::ARMIJO,
-                                         line_search_options,
-                                         &message)));
+  scoped_ptr<LineSearch> line_search(
+      CHECK_NOTNULL(LineSearch::Create(ceres::ARMIJO,
+                                       line_search_options,
+                                       &message)));
   LineSearch::Summary summary;
   line_search_function.Init(x, delta);
   // Try the trust region step.
@@ -407,10 +406,10 @@
       double new_cost = std::numeric_limits<double>::max();
       if (evaluator->Plus(x.data(), delta.data(), x_plus_delta.data())) {
         if (!evaluator->Evaluate(x_plus_delta.data(),
-                                &new_cost,
-                                NULL,
-                                NULL,
-                                NULL)) {
+                                 &new_cost,
+                                 NULL,
+                                 NULL,
+                                 NULL)) {
           LOG(WARNING) << "Step failed to evaluate. "
                        << "Treating it as a step with infinite cost";
           new_cost = numeric_limits<double>::max();