Skip to content

Commit

Permalink
Revert a call to SolveUpperTriangularUsingCholesky.
Browse files Browse the repository at this point in the history
The call to llt in backsubstitute seems to be using one
of the fixed size specializations which is best done with
an inline call to llt/ldlt rather than introducing yet another
variant of the SolverUpperTriangularUsingCholesky and calling it.

Also change the way SolverUpperTriangularUsingCholesky handles
error. It always computes the solution even if it is garbage
and then returns the error code.

This ensures that the previous code that depends on unconditional
computation still works.

Change-Id: Idb1e6efdae9a3775a072e3b87cde02e0bbddb319
  • Loading branch information
sandwichmaker committed Apr 6, 2015
1 parent 59d7f98 commit e2a7169
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 9 deletions.
14 changes: 6 additions & 8 deletions internal/ceres/eigen_dense_cholesky.cc
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,12 @@ SolveUpperTriangularUsingCholesky(int size,
Eigen::LLT<Matrix, Upper> cholesky = lhs.selfadjointView<Upper>().llt();
#endif

if (cholesky.info() == Eigen::Success) {
VectorRef solution(solution_values, size);
if (solution_values == rhs_values) {
cholesky.solveInPlace(solution);
} else {
ConstVectorRef rhs(rhs_values, size);
solution = cholesky.solve(rhs);
}
VectorRef solution(solution_values, size);
if (solution_values == rhs_values) {
cholesky.solveInPlace(solution);
} else {
ConstVectorRef rhs(rhs_values, size);
solution = cholesky.solve(rhs);
}

return cholesky.info();
Expand Down
13 changes: 12 additions & 1 deletion internal/ceres/schur_eliminator_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,18 @@ BackSubstitute(const BlockSparseMatrix* A,
ete.data(), 0, 0, e_block_size, e_block_size);
}

SolveUpperTriangularUsingCholesky(e_block_size, ete.data(), y_ptr, y_ptr);
// On ARM we have experienced significant numerical problems with
// Eigen's LLT implementation. Defining
// CERES_USE_LDLT_FOR_EIGEN_CHOLESKY switches to using the slightly
// more expensive but much more numerically well behaved LDLT
// factorization algorithm.

#ifdef CERES_USE_LDLT_FOR_EIGEN_CHOLESKY
ete.ldlt().solveInPlace(y_block);
#else
ete.llt().solveInPlace(y_block);
#endif

}
}

Expand Down

0 comments on commit e2a7169

Please sign in to comment.