Fix a bug in TrustRegionPreprocessor
TrustRegionPreprocessor was not setting Minimizer::Options::is_constrained.
This meant that the line search for bounds constraints was not being
invoked for bounds constrained problems.
And some minor lint cleanup.
Change-Id: I18852cfaf1b33fd90b7d8c196f2063c128126658
diff --git a/internal/ceres/solver.cc b/internal/ceres/solver.cc
index d49abef..00b8709 100644
--- a/internal/ceres/solver.cc
+++ b/internal/ceres/solver.cc
@@ -67,7 +67,7 @@
ss << string("Solver::Options::" #x " = ") << options.x << ". "; \
ss << string("Solver::Options::" #y " = ") << options.y << ". "; \
ss << "Violated constraint: "; \
- ss << string("Solver::Options::" #x ); \
+ ss << string("Solver::Options::" #x); \
ss << string(#OP " Solver::Options::" #y "."); \
*error = ss.str(); \
return false; \
@@ -77,8 +77,8 @@
#define OPTION_GT(x, y) OPTION_OP(x, y, >);
#define OPTION_LE(x, y) OPTION_OP(x, y, <=);
#define OPTION_LT(x, y) OPTION_OP(x, y, <);
-#define OPTION_LE_OPTION(x, y) OPTION_OP_OPTION(x ,y, <=)
-#define OPTION_LT_OPTION(x, y) OPTION_OP_OPTION(x ,y, <)
+#define OPTION_LE_OPTION(x, y) OPTION_OP_OPTION(x, y, <=)
+#define OPTION_LT_OPTION(x, y) OPTION_OP_OPTION(x, y, <)
bool CommonOptionsAreValid(const Solver::Options& options, string* error) {
OPTION_GE(max_num_iterations, 0);
@@ -246,7 +246,6 @@
if ((options.line_search_direction_type == ceres::BFGS ||
options.line_search_direction_type == ceres::LBFGS) &&
options.line_search_type != ceres::WOLFE) {
-
*error =
string("Invalid configuration: Solver::Options::line_search_type = ")
+ string(LineSearchTypeToString(options.line_search_type))
@@ -331,23 +330,23 @@
internal::OrderingToGroupSizes(options.inner_iteration_ordering.get(),
&(summary->inner_iteration_ordering_given));
- summary->dense_linear_algebra_library_type = options.dense_linear_algebra_library_type;
+ summary->dense_linear_algebra_library_type = options.dense_linear_algebra_library_type; // NOLINT
summary->dogleg_type = options.dogleg_type;
summary->inner_iteration_time_in_seconds = 0.0;
summary->inner_iterations_given = options.use_inner_iterations;
- summary->line_search_direction_type = options.line_search_direction_type;
- summary->line_search_interpolation_type = options.line_search_interpolation_type;
+ summary->line_search_direction_type = options.line_search_direction_type; // NOLINT
+ summary->line_search_interpolation_type = options.line_search_interpolation_type; // NOLINT
summary->line_search_type = options.line_search_type;
summary->linear_solver_type_given = options.linear_solver_type;
summary->max_lbfgs_rank = options.max_lbfgs_rank;
summary->minimizer_type = options.minimizer_type;
- summary->nonlinear_conjugate_gradient_type = options.nonlinear_conjugate_gradient_type;
- summary->num_linear_solver_threads_given = options.num_linear_solver_threads;
+ summary->nonlinear_conjugate_gradient_type = options.nonlinear_conjugate_gradient_type; // NOLINT
+ summary->num_linear_solver_threads_given = options.num_linear_solver_threads; // NOLINT
summary->num_threads_given = options.num_threads;
summary->preconditioner_type_given = options.preconditioner_type;
- summary->sparse_linear_algebra_library_type = options.sparse_linear_algebra_library_type;
- summary->trust_region_strategy_type = options.trust_region_strategy_type;
- summary->visibility_clustering_type = options.visibility_clustering_type;
+ summary->sparse_linear_algebra_library_type = options.sparse_linear_algebra_library_type; // NOLINT
+ summary->trust_region_strategy_type = options.trust_region_strategy_type; // NOLINT
+ summary->visibility_clustering_type = options.visibility_clustering_type; // NOLINT
}
void PostSolveSummarize(const internal::PreprocessedProblem& pp,
@@ -357,11 +356,11 @@
internal::OrderingToGroupSizes(pp.options.inner_iteration_ordering.get(),
&(summary->inner_iteration_ordering_used));
- summary->inner_iterations_used = pp.inner_iteration_minimizer.get() != NULL;
+ summary->inner_iterations_used = pp.inner_iteration_minimizer.get() != NULL; // NOLINT
summary->linear_solver_type_used = pp.options.linear_solver_type;
- summary->num_linear_solver_threads_used = pp.options.num_linear_solver_threads;
+ summary->num_linear_solver_threads_used = pp.options.num_linear_solver_threads; // NOLINT
summary->num_threads_used = pp.options.num_threads;
- summary->preconditioner_type_used = pp.options.preconditioner_type;
+ summary->preconditioner_type_used = pp.options.preconditioner_type; // NOLINT
SetSummaryFinalCost(summary);
@@ -424,7 +423,7 @@
}
}
-} // namespace
+} // namespace
bool Solver::Options::IsValid(string* error) const {
if (!CommonOptionsAreValid(*this, error)) {
diff --git a/internal/ceres/trust_region_preprocessor.cc b/internal/ceres/trust_region_preprocessor.cc
index 9778cda..ba3baa1 100644
--- a/internal/ceres/trust_region_preprocessor.cc
+++ b/internal/ceres/trust_region_preprocessor.cc
@@ -289,6 +289,8 @@
const Solver::Options& options = pp->options;
SetupCommonMinimizerOptions(pp);
+ pp->minimizer_options.is_constrained =
+ pp->reduced_program->IsBoundsConstrained();
pp->minimizer_options.jacobian.reset(pp->evaluator->CreateJacobian());
pp->minimizer_options.inner_iteration_minimizer =
pp->inner_iteration_minimizer;
diff --git a/internal/ceres/trust_region_preprocessor_test.cc b/internal/ceres/trust_region_preprocessor_test.cc
index a97cb96..585767c 100644
--- a/internal/ceres/trust_region_preprocessor_test.cc
+++ b/internal/ceres/trust_region_preprocessor_test.cc
@@ -97,7 +97,7 @@
double x = 3.0;
problem.AddResidualBlock(new FailingCostFunction, NULL, &x);
problem.SetParameterBlockConstant(&x);
- Solver::Options options;
+ Solver::Options options;
TrustRegionPreprocessor preprocessor;
PreprocessedProblem pp;
EXPECT_FALSE(preprocessor.Preprocess(options, &problem, &pp));
@@ -200,13 +200,17 @@
PreprocessForGivenLinearSolverAndVerify(DENSE_SCHUR);
}
-#if defined(CERES_USE_EIGEN_SPARSE) || !defined(CERES_NO_SUITE_SPARSE) || !defined(CERES_NO_CX_SPARSE)
+#if defined(CERES_USE_EIGEN_SPARSE) || \
+ !defined(CERES_NO_SUITE_SPARSE) || \
+ !defined(CERES_NO_CX_SPARSE)
TEST_F(LinearSolverAndEvaluatorCreationTest, SparseNormalCholesky) {
PreprocessForGivenLinearSolverAndVerify(SPARSE_NORMAL_CHOLESKY);
}
#endif
-#if defined(CERES_USE_EIGEN_SPARSE) || !defined(CERES_NO_SUITE_SPARSE) || !defined(CERES_NO_CX_SPARSE)
+#if defined(CERES_USE_EIGEN_SPARSE) || \
+ !defined(CERES_NO_SUITE_SPARSE) || \
+ !defined(CERES_NO_CX_SPARSE)
TEST_F(LinearSolverAndEvaluatorCreationTest, SparseSchur) {
PreprocessForGivenLinearSolverAndVerify(SPARSE_SCHUR);
}
@@ -220,6 +224,21 @@
PreprocessForGivenLinearSolverAndVerify(ITERATIVE_SCHUR);
}
+TEST_F(LinearSolverAndEvaluatorCreationTest, MinimizerIsAwareOfBounds) {
+ problem_.SetParameterLowerBound(&x_, 0, 0.0);
+ Solver::Options options;
+ TrustRegionPreprocessor preprocessor;
+ PreprocessedProblem pp;
+ EXPECT_TRUE(preprocessor.Preprocess(options, &problem_, &pp));
+ EXPECT_EQ(pp.options.linear_solver_type, options.linear_solver_type);
+ EXPECT_EQ(pp.linear_solver_options.type, options.linear_solver_type);
+ EXPECT_EQ(pp.evaluator_options.linear_solver_type,
+ options.linear_solver_type);
+ EXPECT_TRUE(pp.linear_solver.get() != NULL);
+ EXPECT_TRUE(pp.evaluator.get() != NULL);
+ EXPECT_TRUE(pp.minimizer_options.is_constrained);
+}
+
TEST_F(LinearSolverAndEvaluatorCreationTest, SchurTypeSolverWithBadOrdering) {
Solver::Options options;
options.linear_solver_type = DENSE_SCHUR;
