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
diff --git a/internal/ceres/line_search_minimizer.cc b/internal/ceres/line_search_minimizer.cc
index 3e8d18a..ae77a73 100644
--- a/internal/ceres/line_search_minimizer.cc
+++ b/internal/ceres/line_search_minimizer.cc
@@ -207,7 +207,7 @@
int num_line_search_direction_restarts = 0;
while (true) {
- if (!RunCallbacks(options.callbacks, iteration_summary, summary)) {
+ if (!RunCallbacks(options, iteration_summary, summary)) {
break;
}
diff --git a/internal/ceres/minimizer.cc b/internal/ceres/minimizer.cc
index bdb6a11..a20eed5 100644
--- a/internal/ceres/minimizer.cc
+++ b/internal/ceres/minimizer.cc
@@ -37,13 +37,14 @@
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) {
@@ -51,11 +52,13 @@
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";
diff --git a/internal/ceres/minimizer.h b/internal/ceres/minimizer.h
index f9f2b51..f1da3f7 100644
--- a/internal/ceres/minimizer.h
+++ b/internal/ceres/minimizer.h
@@ -186,7 +186,7 @@
bool is_constrained;
};
- static bool RunCallbacks(const vector<IterationCallback*> callbacks,
+ static bool RunCallbacks(const Options& options,
const IterationSummary& iteration_summary,
Solver::Summary* summary);
diff --git a/internal/ceres/minimizer_test.cc b/internal/ceres/minimizer_test.cc
index 1058036..0d8b617 100644
--- a/internal/ceres/minimizer_test.cc
+++ b/internal/ceres/minimizer_test.cc
@@ -44,7 +44,7 @@
}
};
-TEST(MinimizerTest, InitializationCopiesCallbacks) {
+TEST(Minimizer, InitializationCopiesCallbacks) {
FakeIterationCallback callback0;
FakeIterationCallback callback1;
@@ -59,5 +59,42 @@
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
diff --git a/internal/ceres/trust_region_minimizer.cc b/internal/ceres/trust_region_minimizer.cc
index 8f8ec98..4be5619 100644
--- a/internal/ceres/trust_region_minimizer.cc
+++ b/internal/ceres/trust_region_minimizer.cc
@@ -256,7 +256,7 @@
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;
}
