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);
