Skip to content

Commit

Permalink
Remove Solver::Options::use_postordering
Browse files Browse the repository at this point in the history
This was an ill-advised and complicated to interpret option
which offers nothing particularly useful.

Change-Id: Ia7741ed62ef977c96fa52299a884e404bee659ac
  • Loading branch information
sandwichmaker committed May 19, 2022
1 parent 30b4d5d commit 8ba8fbb
Show file tree
Hide file tree
Showing 5 changed files with 5 additions and 48 deletions.
24 changes: 0 additions & 24 deletions docs/source/nnls_solving.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 0 additions & 10 deletions include/ceres/solver.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 1 addition & 7 deletions internal/ceres/reorder_program.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
4 changes: 1 addition & 3 deletions internal/ceres/reorder_program.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
7 changes: 3 additions & 4 deletions internal/ceres/trust_region_preprocessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 8ba8fbb

Please sign in to comment.