Commit Graph

702 Commits

Author SHA1 Message Date
Sameer Agarwal 1ece5a95fb Delete obsolete code
Remove outer product computation code from CompressedRowSparseMatrix.
In the process also remove the crsb_cols and crsb_rows vectors from
the matrix, which were added to carry the block sparsity of the matrix
so that the outer product could be computed fast.

InnerProductComputer and its reliance on BlockSparseMatrix has
rendered all of this code moot.

Change-Id: If3ee0dc8ad4ff79594fd1eebc15a647c4495d726
2017-06-22 00:21:09 -07:00
Sameer Agarwal 08e60379ba Integrate InnerProductComputer
Despite its relative size, this is very significant change
to Ceres.

Why
===

Up till now, when the user chose SPARSE_NORMAL_CHOLESKY,
the Jacobian was evaluated in a CompressedRowSparseMatrix,
which was then use to compute the normal equations which were
passed to a sparse linear algebra library for factorization.

The reason to do this was because in the case of SuiteSparse,
we were able to pass the Jacobian matrix directly without
computing the normal equations and SuiteSparse/CHOLMOD did the
normal equation computation.

This turned out to be slow, so Cheng Wang implemented a high
performance version of the matrix-matrix multiply to compute
the normal equations, and all the sparse linear algebra libraries
now are passed the normal equations.

So that raises the question, as to what the best representation
of the Jacobian which is suitable for the normal equation computation.

Turns out BlockSparseMatrix is ideal. It brings two advantages.

1. Jacobian evaluation into a BlockSparseMatrix is considerably
   faster when using a BlockSparseMatrix than
   CompressedRowSparseMatrix. This is because we save on a bunch
   of memory copies.

2. To make the matrix multiplication fast and use the block structure
   Cheng Wang had to essentially make the CompressedRowSparseMatrix
   carry a bunch of sidecar information about the block sparsity,
   essentially making it behave like a BlockSparseMatrix. The resulting
   code had fairly complicated indexing and complicated the semantics
   of CompressedRowSparseMatrix. The new InnerProductComputer class
   does away with all that and once this CL goes in, I will be able to
   remove all that code and simplify the semantics of
   CompressedRowSparseMatrix.

Changes
=======

1. Use InnerProductComputer in SparseNormalCholeskySolver.
2. Change the evaluator instantiated for SPARSE_NORMAL_CHOLESKY with
   static sparsity inside evaluator.cc
3. The former change necessitates that we change ProblemImpl::Evaluate
   to create the evaluate it needs on its own, because it was
   depending on passing "SPARSE_NORMAL_CHOLESKY" as linear solver type
   to the evaluator factor to get an Evaluator which can use
   CompressedRowSparseMatrix objects for storing the Jacobian.
4. Update the tests for SparseNormalCholeskySolver.
5. Separate out the tests for DynamicSparseNormalCholeskySolver into its
   own file.

Change-Id: I2ef7ef8fbfbb4967d0c1ec2068c1c778248fdf5b
2017-06-21 23:41:36 -07:00
Sameer Agarwal b5b394c738 Add InnerProductComputer
Add a class that given a block sparse matrix m will compute
the product m'*m efficiently.

This code is refactoring and cleanup of the code in
CompressedRowSparseMatrix devoted to computing the inner product.

In that class, the code is mistakenly said to be computing
the outer product. It is also devoted to computing the inner
product of a CompressedRowSparseMatrix with itself.

This code works with BlockSparseMatrix objects instead, which
are simpler to deal with as they are better structured to handle
block sparse matrices.

Change-Id: I920fee1a396bb0fcae9e6f7e46a308c7391d21aa
2017-06-21 23:17:54 -07:00
Sameer Agarwal 04325145c9 Performance improvements to BlockSparseMatrix
Re-allocations only happen if the already allocated buffer is
not large enough.

Change-Id: I5ff170a400e32a0ad64ee7c2e8ab59216db1e51b
2017-06-21 22:01:32 -07:00
Sameer Agarwal 1182fc2884 SPARSE_SCHUR + CX_SPARSE = Faster
There was a bug in the trust region preprocessor where no fill
reducing ordering was computed for the case of SPARSE_SCHUR + CX_SPARSE
but this was not signaled to SchurComplementSolver, so it was using
a naive/natural ordering. To fix this two changes are made:

1. TrustRegionProcessor's logic for signaling the ordering to the
   linear solver has been re-worked. The surrounding code has also
   been re-organized for better readability.
2. In SchurComplementSolver::SolveReducedSystem the row and column
   block structure has been added to the CompressedRowSparseMatrix
   containing the Schur complement so that block AMD can be used.

As a result of these changes the linear solve time for
problem-744-543562-pre.txt has been brought down from 58 seconds to
35 seconds.

Change-Id: I4d82efce05175260f97b1f925f8a1b4a9d650cae
2017-06-20 09:13:02 -07:00
Sameer Agarwal ef31944726 A number of changes to BlockSparseMatrix.
1. Add BlockSparseMatrix::CreateDiagonalMatrix
2. Add BlockSparseMatrix::AppendRows
3. Add BlockSparseMatrix::DeleteRowBlocks
4. Add BlockSparseMatrix::CreateRandomMatrix
5. Add a non-default constructor to Compressedlist.

Change-Id: I7cc7656616d059cef4471335f6d5b636807953e6
2017-06-14 10:57:27 -07:00
Sameer Agarwal 15dc18a532 Change how ifdefs are used in dense_linear_solver_test.
It appears that interspersing ifdefs with macros causes problems
with Visual Studio. This patch changes the way we condition
the tests for dense linear solvers based on whether LAPACK is
available or not.

Change-Id: I306247496265c3551edad6bc8fcec9d4cf09e68d
2017-06-12 21:30:27 -07:00
Sameer Agarwal d90e5a708b Deal with zero sized ranges in Uniform correctly.
Change-Id: I6d5c622869e4795c252fa5c26fffbe4844abc3c9
2017-06-11 16:26:48 -07:00
Sameer Agarwal 59220af761 Remove an unused field from SuiteSparseCholesky
Change-Id: I63128de3e9806347560ab6f0ebee82ead44f2e45
2017-06-11 15:19:35 -07:00
Sameer Agarwal b56d561aab Lint changes from Jim Roseborough
Change-Id: I7d63b1a32416d0361f48990fbf6b8ea83a3abb8e
2017-06-08 16:47:00 -07:00
Sameer Agarwal 7174e65598 Refactor compressed_col_sparse_matrix_utils_test
Remove the dependence on SuiteSparse.
Update the CMakeLists.txt file

https://github.com/ceres-solver/ceres-solver/issues/276

Change-Id: Ifd480d465fb19ad6cd195e1025e0ad2dc4e4a2cc
2017-06-08 05:33:58 -07:00
Sameer Agarwal 1fb83c1a53 symmetric_linear_solver_test -> conjugate_gradients_solver_test
Change-Id: Ie3b7b4e0079a74ae3f9e353cfbda91c4b7cdf988
2017-06-08 05:30:15 -07:00
Sameer Agarwal efa091122d Refactor unsymmetric_linear_solver_test
1. Break up unsymmetric_linear_solver_test into
   a. dense_linear_solver_test which covers DENSE_QR and
      DENSE_NORMAL_CHOLESKY.
   b. sparse_normal_cholesky_solver_test which covers
      SPARSE_NORMAL_CHOLESKY.

2. dense_linear_solver_test has been completely re-written. It now
   uses value parameterized tests for better logging. The number of
   test problems as been increased to 2. Last but not the least
   the actual test of correctness is not based on a golden solution
   computed using another linear solver. We now compute the residual
   and ensure that it is small.

https://github.com/ceres-solver/ceres-solver/issues/279

Change-Id: I9546a43e8ae85c31b2096a99405e47da326755ee
2017-06-08 08:18:37 -04:00
Sameer Agarwal 99aca60b2c Do not create a SparseCholesky object when not needed.
SchurComplementSolver implements a variant of ITERATIVE_SCHUR
when explicit_schur_complement is set to true. In this case
the SparseCholesky object should not be instantiated. Even
though there is no CPU cost, it can be the case that ITERATIVE_SCHUR
is being used when there are not sparse linear algebra libraries
are available, and this can result in a crash.

Change-Id: I349d5f79201782689b3ab0ccc2c5001804b44c7b
2017-06-02 11:08:11 -07:00
Sameer Agarwal 366f4cbb03 More lint changes from William Rucklidge
Change-Id: Ica72b02f9509f1eb1961b531072826519df3fddb
2017-06-01 20:03:43 -07:00
Sameer Agarwal f93ad78f23 Lint changes from William Rucklidge
Change-Id: I54ee9d7b90232bb0139398a78a09cf4035d23ea6
2017-05-31 19:44:23 -07:00
Sameer Agarwal 9a0ef02b8d Lint changes from Jim Roseborough
Change-Id: Ibca11aa9205f5af6180cb2dcf7ceaf1e8724f092
2017-05-30 15:13:45 -07:00
Sameer Agarwal 07c35a1e8c Remove namspace handling code for std::tuple and std::get
gtest defines aliases in ::testing which will do the right thing.

Change-Id: Ic8c803cbda377602d32a1096f57f0a9b3f8bdb0c
2017-05-30 11:29:59 -07:00
Sameer Agarwal 96e908d796 Add an ifdef to handle tr1 namespace in sparse_cholesky_test.cc
Parametric tests in gunit use tuple, which can be in the std::tr1
or the std namespaces depending on the version of STL one is using.

This change adds conditions the choice of namespace on whether
CXX11 mode is enabled or not.

It is entirely possible that we will have to come back and add
detection for this along the lines of shared_ptr.

Change-Id: I7fc85a32cf9f3f3bf30f86d9ba972ac67c6635fb
2017-05-30 02:52:41 +00:00
Sameer Agarwal 01f2414e97 Minor refactoring/cleanup in IterativeSchurComplementSolver
The code for creating and updating preconditioners has been pulled
out in into its own function for better readability.

Change-Id: I5335de3b158a8485cf6d052ee37d0d8fd57145e9
2017-05-30 02:52:25 +00:00
Sameer Agarwal 88dad7e41d Fix the build for dynamic_sparsity.
The addition of crsb_rows and crsb_cols to CompressedRowSparseMatrix
broke the build for problems with dynamic sparsity.

The fix is to remove unnecessarily filling of row_blocks and col_blocks,
which was triggering a check inside CompressedRowSparseMatrix around
block handling, since block structure makes no sense for matrices with
dynamic sparsity anyways.

Also added a test "dynamic_sparsity_test" based on i
examples/ellipse_approximation.cc

Thanks to Richard Stebbing for reporting this.

Change-Id: Ic1d49e97690ac17e0ea2949772271bd915277d68
2017-05-29 19:47:34 -07:00
Sameer Agarwal 97cefd4b35 Fix a memory leak in suitesparse.cc
Change-Id: I6eb8b33d927840d6c3631679de482038d9f3673a
2017-05-24 01:17:03 -07:00
Sameer Agarwal 29c21f5680 Add SparseCholesky
SparseCholesky is an interface to sparse cholesky factorization
routines across sparse linear algebra libraries. Each sparse
linear algebra library is responsible for implementing its own
instance of this interface.

As a result the various places - SparseNormalCholeskySolver,
SparseSchurComplementSolver and VisibilityBasedPreconditioner
are significantly simplified.

Change-Id: I8b465705eae83bba9e1adfffcc741a05c70faf2e
2017-05-24 00:00:25 -07:00
Sameer Agarwal 39388bd2ff Address comments from William Rucklidge
Change-Id: I0c4f686b89798435741c55a5ca45cf000af842dc
2017-05-09 17:23:43 -07:00
Sameer Agarwal 0780d22c54 Remove dead code.
Change-Id: Ic6b2bccb44a97604bf966d37d2ae06f568dd999f
2017-05-08 06:47:44 -07:00
Sameer Agarwal 086ff01aca Matrix generation cleanup
1. Convert a CompressedRowSparseMatrix constructor which
takes a TripletSparseMatrix as input into a factory method
which allows the input to be transposed.

2. Move the random matrix creation routine for CompressedRowSparseMatrix
from being a standalone function to a static method.

3. Add a corresponding random matrix generation static method to
TripletSparseMatrix.

4. Add a new constructor to TripletSparseMatrix, which takes as input
the row, col and values arrays.

Change-Id: Iec7b184646818f432a5e6822bea3b2f3128a82aa
2017-05-01 22:53:21 -07:00
Alex Stewart d72e19d985 Use target_compile_features() to specify C++11 requirement if available.
- Use target_compile_features() to specify the C++11 dependency for
  Ceres if the CXX11 option is enabled and the current CMake version
  supports it (>= 3.1).  Otherwise fall back onto our existing
  target_compile_options() solution if available.
- We prefer the use of target_compile_features() if available as it more
  gracefully handles ‘upgrading’ of the C++ standard in client projects
  that depend upon Ceres, e.g. if the client requires C++14.  The
  current solution may fail to produce the expected result in this case
  as raised in
  https://github.com/ceres-solver/ceres-solver/issues/273.

Change-Id: Ib3cff8d4b9fe93fa6d6b376b4dd53923bb1c4ecc
2017-04-30 14:51:03 +00:00
Sameer Agarwal 1d7185f130 Refactor SparseNormalCholeskySolver
Now that there is a single piece of code doing the outer product
computation for all three sparse linear algebra backends, move
this code one level up the call stack and there by make the actual
per-library solver code shorter and simpler.

Also fix a minor omission in the outer product computation code
where row/column blocks were not being copied over to the
outer product matrix.

Change-Id: I22a7967bdc659385b741901afefa7af312e676e5
2017-04-27 01:45:05 -07:00
Sameer Agarwal 0859fe8a57 Integrate InvertPSDMatrix into the SchurEliminator.
SchurEliminator::Init now takes a bool that tells it whether
it can assume that the diagonal blocks it is inverting can
be assumed to be full rank or not.

This information is then passed onto InvertPSDMatrix.

Change-Id: I26037b6233f2aad5584fed245f631c3959928afe
2017-04-25 10:08:45 -07:00
Sameer Agarwal c9abea1fa0 Add InvertPSDMatrix.
This helper function lets the user compute the inverse or
the pseudo-inverse of a matrix.

Change-Id: Ia257859ad901debce2ea19f47907faf0b7b94759
2017-04-25 01:30:44 +00:00
Sameer Agarwal 14d8297cf9 Refactor Covariance::Options::algorithm_type.
THIS IS AN API BREAKING CHANGE.

Decouple the algorithm from the sparse linear algebra
library being used to perform the computation.

Before this change

Covariance::AlgorithmType had values

DENSE_SVD
EIGEN_SPARSE_QR
SUITE_SPARSE_QR

This has been replaced by two enums now.

Covariance::Options::sparse_linear_algebra_library_type
which can take values EIGEN_SPARSE, SUITE_SPARSE or CX_SPARSE.
The last one is currently not supported.

And Covariance::Options::algorithm_type takes values

DENSE_SVD
SPARSE_QR

This sets the stage for future extensions of the covariance
computation algorithm.

Also as part of this change, the covariance computation chapter
has been made a top level chapter on its own instead of being
buried deep inside the Solving Non-linear Least Squares problem.

Change-Id: Ibfbf60902d8d17694d9ff585047a5a57d329ab22
2017-04-17 09:43:22 -07:00
Sameer Agarwal d04347af43 Delinting from William Rucklidge.
Change-Id: I1b0243d43d55b6c9cdcda9d4b677ec444928b223
2017-04-13 11:47:01 -07:00
Sameer Agarwal a171ba095c Fix an off by one error in the transpose code.
Change-Id: I0ebcf2e8e29dc48376f60c3fa8d6100b20ec105f
2017-04-13 09:47:49 -07:00
Sameer Agarwal 9a6b4a6491 Fix a bug in the transpose code.
Change-Id: I1ce997329c7d59b831b84bf41d470ce686fea566
2017-04-13 07:27:48 -07:00
Sameer Agarwal 3de61fc9c0 Improve comments for TransposeForCompressedRowSparseStructure.
Also make the code a little more safe in its assumptions around
zeroed out arrays.

Change-Id: I5a3588162da496fae2b31a4bf8adef1a02b85ec6
2017-04-13 07:11:58 -07:00
Sameer Agarwal bb5c3bb001 Fix the build.
Change-Id: I87216d4a349fa7cb297c502a90cb54c02b9b4445
2017-04-13 06:20:13 -07:00
Sameer Agarwal 9a4f554b85 Change a size() == 0 to empty
Change-Id: I79c7021781093ab1cc25e6cc9700e2a83169daac
2017-04-12 14:42:00 -07:00
Sameer Agarwal 1f8fa9d26b Various grammar and lint corrections.
Thanks William Rucklidge as usual :)

Change-Id: I787b3a0302bcfd08dc1be165d39b7295e58ff20f
2017-04-12 13:37:28 -07:00
Sameer Agarwal 64a20e7ce2 Remove dead code from CompressedRowSparseMatrix.
SolveLowerTriangularInPlace
SolveLowerTriangularTransposeInPlace

were unused functions which can be removed.

Change-Id: I0fd29c1efae2a0a74666f6e3541473bebc22ae82
2017-04-11 12:10:40 -07:00
Sameer Agarwal 3d0588b24d Remove dynamic sparsity handling from SparseNormalCholeskySolver
Change-Id: I25d1f9dfe14f9cf526ff7aad1a88ca0e711f6caa
2017-04-11 11:59:52 -07:00
Sameer Agarwal 5c2fd0526a Add DynamicSparseNormalCholeskySolver
This code was currently buried under a bool inside SparseNormalCholeskySolver.
Pulling this out in its own solver makes the code simpler more readable
and more performant in the case of SuiteSparse.

Change-Id: I72379ca9ca162abbb83c12f7ee8ff92bc71e772c
2017-04-11 11:55:43 -07:00
Sameer Agarwal 2755fce8d3 Add a storage type to CompressedRowSparseMatrix
By adding an enum to CompressedRowSparseMatrix, which indicates
whether the matrix is unsymmetric, upper or lower triangular
we are able to improve the readability and fix some minor
bugs in the way some matrix manipulation code was being
called.

Thank to William Rucklidge for this suggestion.

Change-Id: I355c90d11cd5d31f5a25741b0bda4fc4583e9095
2017-04-11 11:47:42 -07:00
Sameer Agarwal 5f41a60458 Refactor CreateRandomCompressedRowSparseMatrix
Move it to compressed_row_sparse_matrix.h/cc for upcoming re-use.
Also clean up the tests for ComputeOuterProduct so that they do
not depend on CXSparse anymore and use Eigen instead. This also
makes the test simpler and shorter.

Change-Id: I06bbeb3b0c6a07fb1f3da354ef0abd17d246be9a
2017-04-11 11:33:37 -07:00
Sameer Agarwal afe1382ff7 Add missing <iterator> include.
Change-Id: Ied1199569d952fcdcaa0171893631d01d40eea7f
2017-04-10 11:55:05 -07:00
Cheng Wang 07dbf31eca Optimize J' * J in sparse_normal_cholesky_solver.
1. Add stype to the outerproduct computation to control the output
matrix in upper or lower triangular matrix. For SuiteSparse,
upper triangular matrix is generated. SuiteSparse can directly use
this matrix format for cholesky without matrix transpose overhead.

2. Change the outerproduct computation to block multiplication.  This
reduces the computation complexity for the sort in preprocessing, also
allows formulation of the block outerproduct computation as dense Eigen
block matrix multiplication.

3. Solve 32 Tango problems on Qualcomm MSM8994 Cortex-A53 (1.55GHz)
   before change: 140 seconds
   after change: 131 seconds

Change-Id: I8054114cef911de6a303310a448821ca296e4744
2017-04-10 09:50:34 -07:00
Alex Stewart c123864372 Use Ceres_[SOURCE/BINARY]_DIR not CMAKE_XXX_DIR to support nesting.
- Using Ceres_[SOURCE/BINARY]_DIR (which are defined by CMake when
  project(Ceres) is called, in favour of CMAKE_[SOURCE/BINARY]_DIR
  enables Ceres to be nested within (and built by) a larger CMake
  project (which also contains other projects).
- CMAKE_[SOURCE/BINARY]_DIR always refers to the top-level source
  and binary directories (i.e. the first encountered), as a result if
  Ceres is a nested project within a larger project, these would not
  correctly identify the source/binary directories for Ceres (as they
  would refer to the root project in which Ceres is nested).
- Using Ceres_[SOURCE/BINARY]_DIR should ensure that Ceres always uses
  the correct source/binary directories, irrespective of whether Ceres
  is nested or not.

Change-Id: I62226ea3f6552b1d7e2bdac1aef02f1f489ae55e
2017-04-03 14:47:29 +01:00
Devin Lane f4233598db Add public headers to CMake target for IDEs.
Change-Id: I952c5005689d34bdb7b89820b3aee62f3dcf6fd6
2017-03-29 20:31:19 -06:00
Sameer Agarwal 85c6b5c308 Add a test for LineSearchMinimizer
Ensures that a quadratic function can be minimizer to zero
and the final cost is reported correctly.

Change-Id: Ib03752b627988dc8038566ed1fa444b7d9d2fd2d
2017-03-19 21:12:53 -07:00
Sameer Agarwal e9eb8a324e Fix when LineSearchMinimizer adds the IterationSummary to Solver::Summary
Previously, even when an iteration was successful, the LineSearchMinimizer
would only add the iteration summary to the Summary object if none
of the convergence tests were passed. This could cause iterations with
significant progress in the last iteration to be mis-reported.

The solution would be correct, but the actual cost would be misreported.

This change changes the order of these operations and ensures that
the iteration summary is added whenever the iteration itself is successful.

Thanks to Daniel Weindl for reporting this.

Change-Id: Iff10eccb49d50ad28127f44e149c17fa466db4ae
2017-03-19 00:38:01 -07:00
Alex Stewart 987d3b6b37 Fix ambiguous reference to WARNING when using miniglog.
- When compiling on Clang on OS X with MINIGLOG enabled, name lookup
  finds two ambiguous definitions of WARNING, one in the global
  namespace and one in the google namespace, both defined in the
  miniglog version of logging.h.

Change-Id: I6f1ad7d2750e1ed20ec1ba4574aab599086431df
2017-03-09 11:56:50 +00:00