From 8ba8fbb173db5a1e01feeafe875c1f04839fd97b Mon Sep 17 00:00:00 2001 From: Sameer Agarwal Date: Thu, 19 May 2022 13:27:59 -0700 Subject: [PATCH] Remove Solver::Options::use_postordering This was an ill-advised and complicated to interpret option which offers nothing particularly useful. Change-Id: Ia7741ed62ef977c96fa52299a884e404bee659ac --- docs/source/nnls_solving.rst | 24 --------------------- include/ceres/solver.h | 10 --------- internal/ceres/reorder_program.cc | 8 +------ internal/ceres/reorder_program.h | 4 +--- internal/ceres/trust_region_preprocessor.cc | 7 +++--- 5 files changed, 5 insertions(+), 48 deletions(-) diff --git a/docs/source/nnls_solving.rst b/docs/source/nnls_solving.rst index ce10a11d95..516b261c63 100644 --- a/docs/source/nnls_solving.rst +++ b/docs/source/nnls_solving.rst @@ -1415,30 +1415,6 @@ elimination group [LiSaad]_. This option can only be used with the ``SCHUR_JACOBI`` preconditioner. -.. member:: bool Solver::Options::use_post_ordering - - Default: ``false`` - - Sparse Cholesky factorization algorithms use a fill-reducing - ordering to permute the columns of the Jacobian matrix. There are - two ways of doing this. - - 1. Compute the Jacobian matrix in some order and then have the - factorization algorithm permute the columns of the Jacobian. - - 2. Compute the Jacobian with its columns already permuted. - - The first option incurs a significant memory penalty. The - factorization algorithm has to make a copy of the permuted Jacobian - matrix, thus Ceres pre-permutes the columns of the Jacobian matrix - and generally speaking, there is no performance penalty for doing - so. - - In some rare cases, it is worth using a more complicated reordering - algorithm which has slightly better runtime performance at the - expense of an extra copy of the Jacobian matrix. Setting - ``use_postordering`` to ``true`` enables this tradeoff. - .. member:: bool Solver::Options::dynamic_sparsity Some non-linear least squares problems are symbolically dense but diff --git a/include/ceres/solver.h b/include/ceres/solver.h index 43146b110b..92cb495c23 100644 --- a/include/ceres/solver.h +++ b/include/ceres/solver.h @@ -588,16 +588,6 @@ class CERES_EXPORT Solver { // Jacobian matrix and generally speaking, there is no performance // penalty for doing so. - // TODO(sameeragarwal): Remove this option. It is too obscure and - // there is no clear way of figuring out when this is a useful - // thing to do. - // - // In some rare cases, it is worth using a more complicated - // reordering algorithm which has slightly better runtime - // performance at the expense of an extra copy of the Jacobian - // matrix. Setting use_postordering to true enables this tradeoff. - bool use_postordering = false; - // Some non-linear least squares problems are symbolically dense but // numerically sparse. i.e. at any given state only a small number // of jacobian entries are non-zero, but the position and number of diff --git a/internal/ceres/reorder_program.cc b/internal/ceres/reorder_program.cc index 1f50a39efa..27577254a5 100644 --- a/internal/ceres/reorder_program.cc +++ b/internal/ceres/reorder_program.cc @@ -602,13 +602,7 @@ bool AreJacobianColumnsOrdered( const LinearSolverType linear_solver_type, const PreconditionerType preconditioner_type, const SparseLinearAlgebraLibraryType sparse_linear_algebra_library_type, - const LinearSolverOrderingType linear_solver_ordering_type, - const bool use_postordering, - const bool dynamic_sparsity) { - if (use_postordering || dynamic_sparsity) { - return false; - } - + const LinearSolverOrderingType linear_solver_ordering_type) { if (sparse_linear_algebra_library_type == SUITE_SPARSE) { if (linear_solver_type == SPARSE_NORMAL_CHOLESKY || (linear_solver_type == CGNR && preconditioner_type == SUBSET)) { diff --git a/internal/ceres/reorder_program.h b/internal/ceres/reorder_program.h index 8ae456ec40..ab91825969 100644 --- a/internal/ceres/reorder_program.h +++ b/internal/ceres/reorder_program.h @@ -120,9 +120,7 @@ CERES_NO_EXPORT bool AreJacobianColumnsOrdered( LinearSolverType linear_solver_type, PreconditionerType preconditioner_type, SparseLinearAlgebraLibraryType sparse_linear_algebra_library_type, - LinearSolverOrderingType linear_solver_ordering_type, - bool use_postordering, - bool dynamic_sparsity); + LinearSolverOrderingType linear_solver_ordering_type); } // namespace ceres::internal diff --git a/internal/ceres/trust_region_preprocessor.cc b/internal/ceres/trust_region_preprocessor.cc index ed62aa9d82..a5a986ff95 100644 --- a/internal/ceres/trust_region_preprocessor.cc +++ b/internal/ceres/trust_region_preprocessor.cc @@ -229,12 +229,11 @@ bool SetupLinearSolver(PreprocessedProblem* pp) { } } - if (AreJacobianColumnsOrdered(options.linear_solver_type, + if (!options.dynamic_sparsity && + AreJacobianColumnsOrdered(options.linear_solver_type, options.preconditioner_type, options.sparse_linear_algebra_library_type, - options.linear_solver_ordering_type, - options.use_postordering, - options.dynamic_sparsity)) { + options.linear_solver_ordering_type)) { pp->linear_solver_options.ordering_type = OrderingType::NATURAL; } else { if (options.linear_solver_ordering_type == ceres::AMD) {