Lint changes from William Rucklidge Change-Id: I54ee9d7b90232bb0139398a78a09cf4035d23ea6
diff --git a/examples/ellipse_approximation.cc b/examples/ellipse_approximation.cc index 7b2bf12..f519f0c 100644 --- a/examples/ellipse_approximation.cc +++ b/examples/ellipse_approximation.cc
@@ -272,7 +272,7 @@ class PointToLineSegmentContourCostFunction : public ceres::CostFunction { public: PointToLineSegmentContourCostFunction(const int num_segments, - const Eigen::Vector2d y) + const Eigen::Vector2d& y) : num_segments_(num_segments), y_(y) { // The first parameter is the preimage position. mutable_parameter_block_sizes()->push_back(1); @@ -323,12 +323,12 @@ } static ceres::CostFunction* Create(const int num_segments, - const Eigen::Vector2d y) { + const Eigen::Vector2d& y) { return new PointToLineSegmentContourCostFunction(num_segments, y); } private: - inline double ModuloNumSegments(const double& t) const { + inline double ModuloNumSegments(const double t) const { return t - num_segments_ * floor(t / num_segments_); } @@ -336,8 +336,9 @@ const Eigen::Vector2d y_; }; -struct EuclideanDistanceFunctor { - EuclideanDistanceFunctor(const double& sqrt_weight) +class EuclideanDistanceFunctor { + public: + explicit EuclideanDistanceFunctor(const double& sqrt_weight) : sqrt_weight_(sqrt_weight) {} template <typename T> @@ -347,7 +348,7 @@ return true; } - static ceres::CostFunction* Create(const double& sqrt_weight) { + static ceres::CostFunction* Create(const double sqrt_weight) { return new ceres::AutoDiffCostFunction<EuclideanDistanceFunctor, 2, 2, 2>( new EuclideanDistanceFunctor(sqrt_weight)); }
diff --git a/internal/ceres/cxsparse.cc b/internal/ceres/cxsparse.cc index 9e9c599..11ff943 100644 --- a/internal/ceres/cxsparse.cc +++ b/internal/ceres/cxsparse.cc
@@ -77,13 +77,13 @@ // refers to the scratch space. // // Set x = P * b. - cs_di_ipvec(symbolic_factor->pinv, b, scratch_, num_cols); + CHECK(cs_di_ipvec(symbolic_factor->pinv, b, scratch_, num_cols)); // Set x = L \ x. - cs_di_lsolve(numeric_factor->L, scratch_); + CHECK(cs_di_lsolve(numeric_factor->L, scratch_)); // Set x = L' \ x. - cs_di_ltsolve(numeric_factor->L, scratch_); + CHECK(cs_di_ltsolve(numeric_factor->L, scratch_)); // Set b = P' * x. - cs_di_pvec(symbolic_factor->pinv, scratch_, b, num_cols); + CHECK(cs_di_pvec(symbolic_factor->pinv, scratch_, b, num_cols)); } bool CXSparse::SolveCholesky(cs_di* lhs, double* rhs_and_solution) {
diff --git a/internal/ceres/cxsparse.h b/internal/ceres/cxsparse.h index 2311f4a..1ad79f9 100644 --- a/internal/ceres/cxsparse.h +++ b/internal/ceres/cxsparse.h
@@ -63,7 +63,9 @@ bool SolveCholesky(cs_di* lhs, double* rhs_and_solution); // Solves a linear system given its symbolic and numeric factorization. - void Solve(cs_dis* symbolic_factor, csn* numeric_factor, double* rhs_and_solution); + void Solve(cs_dis* symbolic_factor, + csn* numeric_factor, + double* rhs_and_solution); // Compute the numeric Cholesky factorization of A, given its // symbolic factorization. @@ -133,7 +135,8 @@ int scratch_size_; }; -// Sparse cholesky factorization using CXSparse. +// An implementation of SparseCholesky interface using the CXSparse +// library. class CXSparseCholesky : public SparseCholesky { public: // Factory @@ -142,11 +145,12 @@ // SparseCholesky interface. virtual ~CXSparseCholesky(); virtual CompressedRowSparseMatrix::StorageType StorageType() const; - virtual LinearSolverTerminationType Factorize( - CompressedRowSparseMatrix* lhs, std::string* message); + virtual LinearSolverTerminationType Factorize(CompressedRowSparseMatrix* lhs, + std::string* message); virtual LinearSolverTerminationType Solve(const double* rhs, double* solution, std::string* message); + private: CXSparseCholesky(const OrderingType ordering_type); void FreeSymbolicFactorization(); @@ -161,7 +165,7 @@ } // namespace internal } // namespace ceres -#else // CERES_NO_CXSPARSE +#else // CERES_NO_CXSPARSE typedef void cs_dis;
diff --git a/internal/ceres/dynamic_sparse_normal_cholesky_solver.cc b/internal/ceres/dynamic_sparse_normal_cholesky_solver.cc index 4411223..6d39616 100644 --- a/internal/ceres/dynamic_sparse_normal_cholesky_solver.cc +++ b/internal/ceres/dynamic_sparse_normal_cholesky_solver.cc
@@ -211,7 +211,10 @@ cxsparse.Free(a); event_logger.AddEvent("NormalEquations"); - cxsparse.SolveCholesky(lhs, rhs_and_solution); + if (!cxsparse.SolveCholesky(lhs, rhs_and_solution)) { + summary.termination_type = LINEAR_SOLVER_FAILURE; + summary.message = "CXSparse::SolveCholesky failed"; + } event_logger.AddEvent("Solve"); cxsparse.Free(lhs);
diff --git a/internal/ceres/dynamic_sparsity_test.cc b/internal/ceres/dynamic_sparsity_test.cc index f40fc43..7cf5b34 100644 --- a/internal/ceres/dynamic_sparsity_test.cc +++ b/internal/ceres/dynamic_sparsity_test.cc
@@ -273,7 +273,7 @@ class PointToLineSegmentContourCostFunction : public CostFunction { public: PointToLineSegmentContourCostFunction(const int num_segments, - const Eigen::Vector2d y) + const Eigen::Vector2d& y) : num_segments_(num_segments), y_(y) { // The first parameter is the preimage position. mutable_parameter_block_sizes()->push_back(1); @@ -323,12 +323,12 @@ return true; } - static CostFunction* Create(const int num_segments, const Eigen::Vector2d y) { + static CostFunction* Create(const int num_segments, const Eigen::Vector2d& y) { return new PointToLineSegmentContourCostFunction(num_segments, y); } private: - inline double ModuloNumSegments(const double& t) const { + inline double ModuloNumSegments(const double t) const { return t - num_segments_ * floor(t / num_segments_); } @@ -336,8 +336,9 @@ const Eigen::Vector2d y_; }; -struct EuclideanDistanceFunctor { - EuclideanDistanceFunctor(const double& sqrt_weight) +class EuclideanDistanceFunctor { + public: + explicit EuclideanDistanceFunctor(const double& sqrt_weight) : sqrt_weight_(sqrt_weight) {} template <typename T> @@ -347,7 +348,7 @@ return true; } - static CostFunction* Create(const double& sqrt_weight) { + static CostFunction* Create(const double sqrt_weight) { return new AutoDiffCostFunction<EuclideanDistanceFunctor, 2, 2, 2>( new EuclideanDistanceFunctor(sqrt_weight)); }
diff --git a/internal/ceres/iterative_schur_complement_solver.cc b/internal/ceres/iterative_schur_complement_solver.cc index bf2a3fb..62b8a06 100644 --- a/internal/ceres/iterative_schur_complement_solver.cc +++ b/internal/ceres/iterative_schur_complement_solver.cc
@@ -104,15 +104,16 @@ cg_per_solve_options.r_tolerance = per_solve_options.r_tolerance; cg_per_solve_options.q_tolerance = per_solve_options.q_tolerance; - if (!CreateAndOrUpdatePreconditioner(A, per_solve_options.D)) { - LinearSolver::Summary summary; - summary.num_iterations = 0; - summary.termination_type = LINEAR_SOLVER_FAILURE; - summary.message = "Preconditioner creation or update failed."; - return summary; - } - + CreatePreconditioner(A); if (preconditioner_.get() != NULL) { + if (!preconditioner_->Update(*A, per_solve_options.D)) { + LinearSolver::Summary summary; + summary.num_iterations = 0; + summary.termination_type = LINEAR_SOLVER_FAILURE; + summary.message = "Preconditioner update failed."; + return summary; + } + cg_per_solve_options.preconditioner = preconditioner_.get(); } @@ -131,10 +132,10 @@ return summary; } -bool IterativeSchurComplementSolver::CreateAndOrUpdatePreconditioner( - BlockSparseMatrix* A, double* D) { - if (options_.preconditioner_type == IDENTITY) { - return true; +void IterativeSchurComplementSolver::CreatePreconditioner(BlockSparseMatrix* A) { + if (options_.preconditioner_type == IDENTITY || + preconditioner_.get() != NULL) { + return; } Preconditioner::Options preconditioner_options; @@ -149,28 +150,24 @@ preconditioner_options.f_block_size = options_.f_block_size; preconditioner_options.elimination_groups = options_.elimination_groups; - if (preconditioner_.get() == NULL) { - switch (options_.preconditioner_type) { - case JACOBI: - preconditioner_.reset(new SparseMatrixPreconditionerWrapper( - schur_complement_->block_diagonal_FtF_inverse())); - break; - case SCHUR_JACOBI: - preconditioner_.reset(new SchurJacobiPreconditioner( - *A->block_structure(), preconditioner_options)); - break; - case CLUSTER_JACOBI: - case CLUSTER_TRIDIAGONAL: - preconditioner_.reset(new VisibilityBasedPreconditioner( - *A->block_structure(), preconditioner_options)); - break; - default: - LOG(FATAL) << "Unknown Preconditioner Type"; - } + switch (options_.preconditioner_type) { + case JACOBI: + preconditioner_.reset(new SparseMatrixPreconditionerWrapper( + schur_complement_->block_diagonal_FtF_inverse())); + break; + case SCHUR_JACOBI: + preconditioner_.reset(new SchurJacobiPreconditioner( + *A->block_structure(), preconditioner_options)); + break; + case CLUSTER_JACOBI: + case CLUSTER_TRIDIAGONAL: + preconditioner_.reset(new VisibilityBasedPreconditioner( + *A->block_structure(), preconditioner_options)); + break; + default: + LOG(FATAL) << "Unknown Preconditioner Type"; } - - return preconditioner_->Update(*A, D); -} +}; } // namespace internal } // namespace ceres
diff --git a/internal/ceres/iterative_schur_complement_solver.h b/internal/ceres/iterative_schur_complement_solver.h index ed99f2d..ffcfd8d 100644 --- a/internal/ceres/iterative_schur_complement_solver.h +++ b/internal/ceres/iterative_schur_complement_solver.h
@@ -79,7 +79,7 @@ const LinearSolver::PerSolveOptions& options, double* x); - bool CreateAndOrUpdatePreconditioner(BlockSparseMatrix* A, double* D); + void CreatePreconditioner(BlockSparseMatrix* A); LinearSolver::Options options_; scoped_ptr<internal::ImplicitSchurComplement> schur_complement_;
diff --git a/internal/ceres/sparse_cholesky.h b/internal/ceres/sparse_cholesky.h index 07d328d..d77cccd 100644 --- a/internal/ceres/sparse_cholesky.h +++ b/internal/ceres/sparse_cholesky.h
@@ -47,7 +47,7 @@ // // Instances of SparseCholesky are expected to cache the symbolic // factorization of the linear system. They do this on the first call -// to Factorize of FactorAndSolve. Subsequent calls to Factorize and +// to Factorize or FactorAndSolve. Subsequent calls to Factorize and // FactorAndSolve are expected to have the same sparsity structure. // // Example usage: @@ -85,7 +85,7 @@ // strategy used. virtual CompressedRowSparseMatrix::StorageType StorageType() const = 0; - // Compute the numeric factorization of the given matrix. If this + // Computes the numeric factorization of the given matrix. If this // is the first call to Factorize, first the symbolic factorization // will be computed and cached and the numeric factorization will be // computed based on that. @@ -96,7 +96,7 @@ virtual LinearSolverTerminationType Factorize( CompressedRowSparseMatrix* lhs, std::string* message) = 0; - // Compute the solution to the equation + // Computes the solution to the equation // // lhs * solution = rhs // @@ -106,7 +106,7 @@ std::string* message) = 0; // Convenience method which combines a call to Factorize and - // Solve. Solve is only called is Factorize returns + // Solve. Solve is only called if Factorize returns // LINEAR_SOLVER_SUCCESS. virtual LinearSolverTerminationType FactorAndSolve( CompressedRowSparseMatrix* lhs,