Lint comments from William Rucklidge. Also some minor refactoring of the trust_region_preprocessor_test.cc Change-Id: Ica28002254c95722faf93a7ef35bf3deab557f0b
diff --git a/internal/ceres/preprocessor.h b/internal/ceres/preprocessor.h index 703ba9d..7a875b3 100644 --- a/internal/ceres/preprocessor.h +++ b/internal/ceres/preprocessor.h
@@ -51,7 +51,7 @@ // Given a Problem object and a Solver::Options object indicating the // configuration of the solver, the job of the Preprocessor is to // analyze the Problem and perform the setup needed to solve it using -// the desired Minimization algorithm. The setup involves, removing +// the desired Minimization algorithm. The setup involves removing // redundancies in the input problem (inactive parameter and residual // blocks), finding fill reducing orderings as needed, configuring and // creating various objects needed by the Minimizer to solve the @@ -105,7 +105,8 @@ // Common functions used by various preprocessors. // If OpenMP support is not available and user has requested more than -// one threads, then set the *_num_threads options as needed to 1. +// one threads, then set the *_num_threads options as needed to one +// thread. void ChangeNumThreadsIfNeeded(Solver::Options* options); // Extract the effective parameter vector from the preprocessed
diff --git a/internal/ceres/solver_impl.cc b/internal/ceres/solver_impl.cc index 41dbcde..67bccda 100644 --- a/internal/ceres/solver_impl.cc +++ b/internal/ceres/solver_impl.cc
@@ -111,7 +111,8 @@ trust_region_strategy_options.trust_region_strategy_type = options.trust_region_strategy_type; trust_region_strategy_options.dogleg_type = options.dogleg_type; - minimizer_options.trust_region_strategy.reset(TrustRegionStrategy::Create(trust_region_strategy_options)); + minimizer_options.trust_region_strategy.reset( + TrustRegionStrategy::Create(trust_region_strategy_options)); TrustRegionMinimizer minimizer; double minimizer_start_time = WallTimeInSeconds();
diff --git a/internal/ceres/trust_region_minimizer.cc b/internal/ceres/trust_region_minimizer.cc index 52e3b23..926bced 100644 --- a/internal/ceres/trust_region_minimizer.cc +++ b/internal/ceres/trust_region_minimizer.cc
@@ -131,7 +131,8 @@ Evaluator* evaluator = CHECK_NOTNULL(options_.evaluator.get()); SparseMatrix* jacobian = CHECK_NOTNULL(options_.jacobian.get()); - TrustRegionStrategy* strategy = CHECK_NOTNULL(options_.trust_region_strategy.get()); + TrustRegionStrategy* strategy = + CHECK_NOTNULL(options_.trust_region_strategy.get()); const bool is_not_silent = !options.is_silent;
diff --git a/internal/ceres/trust_region_minimizer_test.cc b/internal/ceres/trust_region_minimizer_test.cc index 94be541..ae32e96 100644 --- a/internal/ceres/trust_region_minimizer_test.cc +++ b/internal/ceres/trust_region_minimizer_test.cc
@@ -227,8 +227,10 @@ minimizer_options.gradient_tolerance = 1e-26; minimizer_options.function_tolerance = 1e-26; minimizer_options.parameter_tolerance = 1e-26; - minimizer_options.evaluator.reset(new PowellEvaluator2<col1, col2, col3, col4>); - minimizer_options.jacobian.reset(minimizer_options.evaluator->CreateJacobian()); + minimizer_options.evaluator.reset( + new PowellEvaluator2<col1, col2, col3, col4>); + minimizer_options.jacobian.reset( + minimizer_options.evaluator->CreateJacobian()); TrustRegionStrategy::Options trust_region_strategy_options; trust_region_strategy_options.trust_region_strategy_type = strategy_type;
diff --git a/internal/ceres/trust_region_preprocessor.cc b/internal/ceres/trust_region_preprocessor.cc index 48b58c9..9778cda 100644 --- a/internal/ceres/trust_region_preprocessor.cc +++ b/internal/ceres/trust_region_preprocessor.cc
@@ -139,7 +139,7 @@ // If the user has not supplied a linear solver ordering, then we // assume that they are giving all the freedom to us in choosing // the best possible ordering. This intent can be indicated by - // putting all the parameter block in the same elimination group. + // putting all the parameter blocks in the same elimination group. options.linear_solver_ordering.reset( CreateDefaultLinearSolverOrdering(*pp->reduced_program)); } else {
diff --git a/internal/ceres/trust_region_preprocessor_test.cc b/internal/ceres/trust_region_preprocessor_test.cc index cfa22fe..a97cb96 100644 --- a/internal/ceres/trust_region_preprocessor_test.cc +++ b/internal/ceres/trust_region_preprocessor_test.cc
@@ -167,7 +167,8 @@ problem_.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y_, &z_); } - void Run(const LinearSolverType linear_solver_type) { + void PreprocessForGivenLinearSolverAndVerify( + const LinearSolverType linear_solver_type) { Solver::Options options; options.linear_solver_type = linear_solver_type; TrustRegionPreprocessor preprocessor; @@ -188,75 +189,61 @@ }; TEST_F(LinearSolverAndEvaluatorCreationTest, DenseQR) { - Run(DENSE_QR); + PreprocessForGivenLinearSolverAndVerify(DENSE_QR); } TEST_F(LinearSolverAndEvaluatorCreationTest, DenseNormalCholesky) { - Run(DENSE_NORMAL_CHOLESKY); + PreprocessForGivenLinearSolverAndVerify(DENSE_NORMAL_CHOLESKY); } TEST_F(LinearSolverAndEvaluatorCreationTest, DenseSchur) { - Run(DENSE_SCHUR); + PreprocessForGivenLinearSolverAndVerify(DENSE_SCHUR); } #if defined(CERES_USE_EIGEN_SPARSE) || !defined(CERES_NO_SUITE_SPARSE) || !defined(CERES_NO_CX_SPARSE) TEST_F(LinearSolverAndEvaluatorCreationTest, SparseNormalCholesky) { - Run(SPARSE_NORMAL_CHOLESKY); + PreprocessForGivenLinearSolverAndVerify(SPARSE_NORMAL_CHOLESKY); } #endif #if defined(CERES_USE_EIGEN_SPARSE) || !defined(CERES_NO_SUITE_SPARSE) || !defined(CERES_NO_CX_SPARSE) TEST_F(LinearSolverAndEvaluatorCreationTest, SparseSchur) { - Run(SPARSE_SCHUR); + PreprocessForGivenLinearSolverAndVerify(SPARSE_SCHUR); } #endif TEST_F(LinearSolverAndEvaluatorCreationTest, CGNR) { - Run(CGNR); + PreprocessForGivenLinearSolverAndVerify(CGNR); } TEST_F(LinearSolverAndEvaluatorCreationTest, IterativeSchur) { - Run(ITERATIVE_SCHUR); + PreprocessForGivenLinearSolverAndVerify(ITERATIVE_SCHUR); } -TEST(TrustRegionPreprocessor, SchurTypeSolverWithBadOrdering) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - +TEST_F(LinearSolverAndEvaluatorCreationTest, SchurTypeSolverWithBadOrdering) { Solver::Options options; options.linear_solver_type = DENSE_SCHUR; options.linear_solver_ordering.reset(new ParameterBlockOrdering); - options.linear_solver_ordering->AddElementToGroup(&x, 0); - options.linear_solver_ordering->AddElementToGroup(&y, 0); - options.linear_solver_ordering->AddElementToGroup(&z, 1); + options.linear_solver_ordering->AddElementToGroup(&x_, 0); + options.linear_solver_ordering->AddElementToGroup(&y_, 0); + options.linear_solver_ordering->AddElementToGroup(&z_, 1); TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_FALSE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_FALSE(preprocessor.Preprocess(options, &problem_, &pp)); } -TEST(TrustRegionPreprocessor, SchurTypeSolverWithGoodOrdering) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - +TEST_F(LinearSolverAndEvaluatorCreationTest, SchurTypeSolverWithGoodOrdering) { Solver::Options options; options.linear_solver_type = DENSE_SCHUR; options.linear_solver_ordering.reset(new ParameterBlockOrdering); - options.linear_solver_ordering->AddElementToGroup(&x, 0); - options.linear_solver_ordering->AddElementToGroup(&z, 0); - options.linear_solver_ordering->AddElementToGroup(&y, 1); + options.linear_solver_ordering->AddElementToGroup(&x_, 0); + options.linear_solver_ordering->AddElementToGroup(&z_, 0); + options.linear_solver_ordering->AddElementToGroup(&y_, 1); TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_TRUE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_TRUE(preprocessor.Preprocess(options, &problem_, &pp)); EXPECT_EQ(pp.options.linear_solver_type, DENSE_SCHUR); EXPECT_EQ(pp.linear_solver_options.type, DENSE_SCHUR); EXPECT_EQ(pp.evaluator_options.linear_solver_type, DENSE_SCHUR); @@ -264,26 +251,21 @@ EXPECT_TRUE(pp.evaluator.get() != NULL); } -TEST(TrustRegionPreprocessor, SchurTypeSolverWithEmptyFirstEliminationGroup) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - problem.SetParameterBlockConstant(&x); - problem.SetParameterBlockConstant(&z); +TEST_F(LinearSolverAndEvaluatorCreationTest, + SchurTypeSolverWithEmptyFirstEliminationGroup) { + problem_.SetParameterBlockConstant(&x_); + problem_.SetParameterBlockConstant(&z_); Solver::Options options; options.linear_solver_type = DENSE_SCHUR; options.linear_solver_ordering.reset(new ParameterBlockOrdering); - options.linear_solver_ordering->AddElementToGroup(&x, 0); - options.linear_solver_ordering->AddElementToGroup(&z, 0); - options.linear_solver_ordering->AddElementToGroup(&y, 1); + options.linear_solver_ordering->AddElementToGroup(&x_, 0); + options.linear_solver_ordering->AddElementToGroup(&z_, 0); + options.linear_solver_ordering->AddElementToGroup(&y_, 1); TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_TRUE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_TRUE(preprocessor.Preprocess(options, &problem_, &pp)); EXPECT_EQ(pp.options.linear_solver_type, DENSE_QR); EXPECT_EQ(pp.linear_solver_options.type, DENSE_QR); EXPECT_EQ(pp.evaluator_options.linear_solver_type, DENSE_QR); @@ -291,25 +273,20 @@ EXPECT_TRUE(pp.evaluator.get() != NULL); } -TEST(TrustRegionPreprocessor, SchurTypeSolverWithEmptySecondEliminationGroup) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - problem.SetParameterBlockConstant(&y); +TEST_F(LinearSolverAndEvaluatorCreationTest, + SchurTypeSolverWithEmptySecondEliminationGroup) { + problem_.SetParameterBlockConstant(&y_); Solver::Options options; options.linear_solver_type = DENSE_SCHUR; options.linear_solver_ordering.reset(new ParameterBlockOrdering); - options.linear_solver_ordering->AddElementToGroup(&x, 0); - options.linear_solver_ordering->AddElementToGroup(&z, 0); - options.linear_solver_ordering->AddElementToGroup(&y, 1); + options.linear_solver_ordering->AddElementToGroup(&x_, 0); + options.linear_solver_ordering->AddElementToGroup(&z_, 0); + options.linear_solver_ordering->AddElementToGroup(&y_, 1); TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_TRUE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_TRUE(preprocessor.Preprocess(options, &problem_, &pp)); EXPECT_EQ(pp.options.linear_solver_type, DENSE_SCHUR); EXPECT_EQ(pp.linear_solver_options.type, DENSE_SCHUR); EXPECT_EQ(pp.evaluator_options.linear_solver_type, DENSE_SCHUR); @@ -317,7 +294,7 @@ EXPECT_TRUE(pp.evaluator.get() != NULL); } -TEST(TrustRegionProcessor, InnerIterationsWithOneParameterBlock) { +TEST(TrustRegionPreprocessorTest, InnerIterationsWithOneParameterBlock) { ProblemImpl problem; double x = 1.0; problem.AddResidualBlock(new DummyCostFunction<1, 1>, NULL, &x); @@ -333,63 +310,44 @@ EXPECT_TRUE(pp.inner_iteration_minimizer.get() == NULL); } -TEST(TrustRegionProcessor, InnerIterationsWithTwoParameterBlocks) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - +TEST_F(LinearSolverAndEvaluatorCreationTest, + InnerIterationsWithTwoParameterBlocks) { Solver::Options options; options.use_inner_iterations = true; TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_TRUE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_TRUE(preprocessor.Preprocess(options, &problem_, &pp)); EXPECT_TRUE(pp.linear_solver.get() != NULL); EXPECT_TRUE(pp.evaluator.get() != NULL); EXPECT_TRUE(pp.inner_iteration_minimizer.get() != NULL); } -TEST(TrustRegionProcessor, InvalidInnerIterationsOrdering) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - +TEST_F(LinearSolverAndEvaluatorCreationTest, + InvalidInnerIterationsOrdering) { Solver::Options options; options.use_inner_iterations = true; options.inner_iteration_ordering.reset(new ParameterBlockOrdering); - options.inner_iteration_ordering->AddElementToGroup(&x, 0); - options.inner_iteration_ordering->AddElementToGroup(&z, 0); - options.inner_iteration_ordering->AddElementToGroup(&y, 0); + options.inner_iteration_ordering->AddElementToGroup(&x_, 0); + options.inner_iteration_ordering->AddElementToGroup(&z_, 0); + options.inner_iteration_ordering->AddElementToGroup(&y_, 0); TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_FALSE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_FALSE(preprocessor.Preprocess(options, &problem_, &pp)); } -TEST(TrustRegionProcessor, ValidInnerIterationsOrdering) { - ProblemImpl problem; - double x = 1.0; - double y = 1.0; - double z = 1.0; - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &x, &y); - problem.AddResidualBlock(new DummyCostFunction<1, 1, 1>, NULL, &y, &z); - +TEST_F(LinearSolverAndEvaluatorCreationTest, ValidInnerIterationsOrdering) { Solver::Options options; options.use_inner_iterations = true; options.inner_iteration_ordering.reset(new ParameterBlockOrdering); - options.inner_iteration_ordering->AddElementToGroup(&x, 0); - options.inner_iteration_ordering->AddElementToGroup(&z, 0); - options.inner_iteration_ordering->AddElementToGroup(&y, 1); + options.inner_iteration_ordering->AddElementToGroup(&x_, 0); + options.inner_iteration_ordering->AddElementToGroup(&z_, 0); + options.inner_iteration_ordering->AddElementToGroup(&y_, 1); TrustRegionPreprocessor preprocessor; PreprocessedProblem pp; - EXPECT_TRUE(preprocessor.Preprocess(options, &problem, &pp)); + EXPECT_TRUE(preprocessor.Preprocess(options, &problem_, &pp)); EXPECT_TRUE(pp.linear_solver.get() != NULL); EXPECT_TRUE(pp.evaluator.get() != NULL); EXPECT_TRUE(pp.inner_iteration_minimizer.get() != NULL);