Fix a bug in Minimizer::RunCallbacks.

Solver::Summary::message was not being updated when the solver
terminated because of a user's iteration callback indicating
success or failure.

Thanks to Sergey Sharybin for reporting this.

Change-Id: I27e6e5eed086920ddf765461b0159417ac79d7b3

internal/ceres/line_search_minimizer.cc

@@ -207,7 +207,7 @@ void LineSearchMinimizer::Minimize(const Minimizer::Options& options,
   int num_line_search_direction_restarts = 0;
 
   while (true) {
-    if (!RunCallbacks(options.callbacks, iteration_summary, summary)) {
+    if (!RunCallbacks(options, iteration_summary, summary)) {
       break;
     }
 

internal/ceres/minimizer.cc

@@ -37,25 +37,28 @@ namespace internal {
 
 Minimizer::~Minimizer() {}
 
-bool Minimizer::RunCallbacks(const vector<IterationCallback*> callbacks,
+bool Minimizer::RunCallbacks(const Minimizer::Options& options,
                              const IterationSummary& iteration_summary,
                              Solver::Summary* summary) {
+  const bool is_not_silent = !options.is_silent;
   CallbackReturnType status = SOLVER_CONTINUE;
   int i = 0;
-  while (status == SOLVER_CONTINUE && i < callbacks.size()) {
-    status = (*callbacks[i])(iteration_summary);
+  while (status == SOLVER_CONTINUE && i < options.callbacks.size()) {
+    status = (*options.callbacks[i])(iteration_summary);
     ++i;
   }
   switch (status) {
     case SOLVER_CONTINUE:
       return true;
     case SOLVER_TERMINATE_SUCCESSFULLY:
       summary->termination_type = USER_SUCCESS;
-      VLOG(1) << "Terminating: User callback returned USER_SUCCESS.";
+      summary->message = "User callback returned SOLVER_TERMINATE_SUCCESSFULLY.";
+      VLOG_IF(is_not_silent, 1) << "Terminating: " << summary->message;
       return false;
     case SOLVER_ABORT:
       summary->termination_type = USER_FAILURE;
-      VLOG(1) << "Terminating: User callback returned  USER_ABORT.";
+      summary->message = "User callback returned SOLVER_ABORT.";
+      VLOG_IF(is_not_silent, 1) << "Terminating: " << summary->message;
       return false;
     default:
       LOG(FATAL) << "Unknown type of user callback status";

internal/ceres/minimizer.h

@@ -186,7 +186,7 @@ class Minimizer {
     bool is_constrained;
   };
 
-  static bool RunCallbacks(const vector<IterationCallback*> callbacks,
+  static bool RunCallbacks(const Options& options,
                            const IterationSummary& iteration_summary,
                            Solver::Summary* summary);
 

internal/ceres/minimizer_test.cc

@@ -44,7 +44,7 @@ class FakeIterationCallback : public IterationCallback {
   }
 };
 
-TEST(MinimizerTest, InitializationCopiesCallbacks) {
+TEST(Minimizer, InitializationCopiesCallbacks) {
   FakeIterationCallback callback0;
   FakeIterationCallback callback1;
 
@@ -59,5 +59,42 @@ TEST(MinimizerTest, InitializationCopiesCallbacks) {
   EXPECT_EQ(minimizer_options.callbacks[1], &callback1);
 }
 
+class AbortingIterationCallback : public IterationCallback {
+ public:
+  virtual ~AbortingIterationCallback() {}
+  virtual CallbackReturnType operator()(const IterationSummary& summary) {
+    return SOLVER_ABORT;
+  }
+};
+
+TEST(Minimizer, UserAbortUpdatesSummaryMessage) {
+  AbortingIterationCallback callback;
+  Solver::Options solver_options;
+  solver_options.callbacks.push_back(&callback);
+  Minimizer::Options minimizer_options(solver_options);
+  Solver::Summary summary;
+  Minimizer::RunCallbacks(minimizer_options, IterationSummary(), &summary);
+  EXPECT_EQ(summary.message, "User callback returned SOLVER_ABORT.");
+}
+
+class SucceedingIterationCallback : public IterationCallback {
+ public:
+  virtual ~SucceedingIterationCallback() {}
+  virtual CallbackReturnType operator()(const IterationSummary& summary) {
+    return SOLVER_TERMINATE_SUCCESSFULLY;
+  }
+};
+
+TEST(Minimizer, UserSuccessUpdatesSummaryMessage) {
+  SucceedingIterationCallback callback;
+  Solver::Options solver_options;
+  solver_options.callbacks.push_back(&callback);
+  Minimizer::Options minimizer_options(solver_options);
+  Solver::Summary summary;
+  Minimizer::RunCallbacks(minimizer_options, IterationSummary(), &summary);
+  EXPECT_EQ(summary.message,
+            "User callback returned SOLVER_TERMINATE_SUCCESSFULLY.");
+}
+
 }  // namespace internal
 }  // namespace ceres

internal/ceres/trust_region_minimizer.cc

@@ -256,7 +256,7 @@ void TrustRegionMinimizer::Minimize(const Minimizer::Options& options,
   bool inner_iterations_are_enabled = options.inner_iteration_minimizer != NULL;
   while (true) {
     bool inner_iterations_were_useful = false;
-    if (!RunCallbacks(options.callbacks, iteration_summary, summary)) {
+    if (!RunCallbacks(options, iteration_summary, summary)) {
       return;
     }