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;