Commit Graph

475 Commits

Author SHA1 Message Date
Sameer Agarwal 62a8d64453 Expand check for lack of a sparse linear algebra library.
The LinearSolver factory was creating a NULL linear solver
if only Eigen's sparse linear algebra backend was available.

Thanks to Michael Samples and Domink Reitzle for reporting this.

Change-Id: I35e3a6c0fd0da2a31934adb5dfe4cad29577cc73
2014-08-28 09:46:10 -07:00
Sameer Agarwal 12eb389b4e Fix Eigen Row/ColMajor bug in NumericDiffCostFunction.
If the parameter block size is 1, asking Eigen to create
a row-major matrix triggers a compile time error. Previously
we were handling the case where the number of rows in the
jacobian block was known statically, but the problem is present
when the nummber of rows is dynamic.

This CL fixes this problem.

Thanks to Dominik Reitzle for reporting this.

Change-Id: I99c3eec3558e66ebf4efa51c4dee8ce292ffe0c1
2014-08-28 16:34:22 +00:00
Alex Stewart 6c25185bb1 Fix crash in Covariance if # threads > 1 requested without OpenMP.
- Previously if options.num_threads > 1 was given to Covariance compiled
  without OpenMP, a CHECK() would be triggered in program_evalutor.

Change-Id: Iaade4f5ed5326b0c59a7014c750c41ee026e1124
2014-08-28 16:07:51 +01:00
Sameer Agarwal 6f89d850fb Further build breakage fixes.
1. Allow the minimum number of linear solver iterations to be zero.
2. Fix conjugate gradients solver's iteration loop to be sane again.

Change-Id: I8594815fec940c2b30e28eb58ec5d8baacf13dae
2014-08-27 11:51:50 -07:00
Sameer Agarwal dd596d0f0d Fix the broken build.
Change-Id: I083cf1cca1bf4cca956193022d450364e73f833a
2014-08-27 11:26:50 -07:00
Sameer Agarwal d906afae22 A number of bug fixes.
1. Fix a build breakage in graph_test.
2. Respect Solver::Options::min_num_linear_solver_iterations in
   conjugate_gradients_solver.cc

Thanks to Johannes Schönberger for reporting these.

Change-Id: Ib32e3929bf5d92dd576ae5b53d4d88797095136e
2014-08-25 22:32:38 -07:00
Sameer Agarwal dab955928c Add an unweighted graph.
Rename Graph -> WeightedGraph.
Add a new Graph class, which is cheaper to construct and
work with if the weights are not needed.

This cuts down the cost of building the Hessian graph
significantly.

Change-Id: Id0cfc81dd2c0bb5ff8f63a1b55aa133c53c0c869
2014-08-25 09:51:35 -07:00
Björn Piltz 20de0a7793 Fixed Malformed regex
I got the following error with MSVC:
Syntax error at index 9 in simple regular expression "NumGroups()": '(' is unsupported.

Change-Id: Id1952831d81d3eb5d73bbed8c311914c4c8ab51f
2014-08-25 17:05:54 +02:00
Björn Piltz ccf8aea988 Fixed MSVC error C2124: divide or mod by zero
Alternatively, if quiet_NaN is not available on all platforms a workaround would be:
    volatile double zero = 0.0;
    double x = 1.0/zero;
The 'volatile' is needed to shut up "warning C4723: potential divide by 0".

Change-Id: If2bbdab8540595aa2e0079e1eb6b6fed6d4a6ef7
2014-08-25 16:16:01 +02:00
Sameer Agarwal 8de27be218 Fix a bug in TrustRegionPreprocessor
TrustRegionPreprocessor was not setting Minimizer::Options::is_constrained.
This meant that the line search for bounds constraints was not being
invoked for bounds constrained problems.

And some minor lint cleanup.

Change-Id: I18852cfaf1b33fd90b7d8c196f2063c128126658
2014-08-19 08:39:48 -07:00
Sameer Agarwal 1745dd615b Refactor SolverImpl.
Replace SolverImpl with

 a. A minimizer specific preprocessor class.
 b. A generic Solve function inside solver.cc
 c. Presummarize and Postsummarize functions to handle
    updates to the summary object.

The existing SolverImpl class was a mixture of the above three
things and was increasingly complicated code to follow. This change,
breaks it into its three separate constituents, with the aims of
better separation of concerns and thus better testability and
reliability.

The call to Solver::Solve() now consists of

1. Presummarize - summarize the given state of the problem and solver
   options.
2. Preprocess - Setup everything that is needed to call the minimizer.
   This includes, removing redundant parameter and residual blocks,
   setting up the reordering for the linear solver, creating the
   linear solver, evaluator, inner iteration minimizer etc.
3. Minimize.
4. Post summarize - summarize the result of the preprocessing and the
   solve.

Change-Id: I80f35cfc9f2cbf78f1df4aceace27075779d8a3a
2014-08-18 13:31:31 -07:00
Sameer Agarwal bd90384226 Lint comments from William Rucklidge.
Also some minor refactoring of the trust_region_preprocessor_test.cc

Change-Id: Ica28002254c95722faf93a7ef35bf3deab557f0b
2014-08-18 11:27:06 -07:00
Sameer Agarwal 3150321db4 Preprocessor for the LineSearchMinimizer.
Change-Id: Ieb5dfe1c0b96ef323c1130edd0c3a8a8b2c644cc
2014-08-12 23:00:07 -07:00
Sameer Agarwal f7da411ef0 Preprocessor for the TrustRegionMinimizer.
1. Base class for preprocessors.
2. A preprocessor for problems that will be solved using
   the trust region minimizer.
3. Added sanity tests to the program reordering options
   for Schur type linear solvers.
4. Tests for the TrustRegionPreprocessor.

Change-Id: I88cd926f0053bbbf2bd6b11e03ec55b8bf473cf1
2014-08-12 21:58:25 -07:00
Alex Stewart 54893ba523 Add missing #include of <limits> for loss functions.
Change-Id: Id632451429e03031a1533a9be795270debc70706
2014-08-11 19:04:18 +01:00
Sameer Agarwal 4a2a888905 Change ownership of pointers in Minimizer::Options.
This is a intermediate change to clean things up
in preparation for a broader refactoring of the SolverImpl.

Essentially we are replacing raw pointers in Minimizer::Options
with shared_ptr objects. For now this only makes things a bit
more complicated looking inside solver_impl.cc, but going
forward this will lead to considerable simplifications in
tracking ownership of various pointers.

Change-Id: I21db8fc6763c29b0d15e834d7c968a0f514042a0
2014-08-07 20:14:12 +00:00
Sameer Agarwal 0d4e3bd664 GradientCheckingProblem's parameter blocks are initialized correctly.
Ensure that when a new problem object is constructed for validing
gradients, the parameter blocks have their data pointers point to
the user's parameter blocks.

We used to do this inside solver_impl.cc, but doing this at
construction is the right thing to do.

Change-Id: I3bfdc89bb0027c8d67cde937e8f2fa385d89c30c
2014-08-07 12:22:48 -07:00
Sameer Agarwal cfb36463f9 Small fixes from William Rucklidge.
Change-Id: I0be52f0f1e53cedccffe4807dc664a2f3fb4a8e4
2014-08-05 14:42:33 -07:00
Sameer Agarwal 9a41132a05 Small changes from Jim Roseborough.
Change-Id: Ic8b19ea5c5f4f8fd782eb4420b30514153087d18
2014-08-05 05:49:27 +00:00
Sameer Agarwal a521fc3afc Simplify, cleanup and instrument SchurComplementSolver.
The instrumentation revealed that EIGEN_SPARSE can be upto
an order of magnitude slower than CX_SPARSE on some bundle
adjustment problems.

The problem comes down to the quality of AMD ordering that
CXSparse/Eigen implements. It does particularly badly
on the Schur complement. In the CXSparse implementation
we got around this by considering the block sparsity structure
and computing the AMD ordering on it and lifting it to the
full matrix.

This is currently not possible with the release version of
Eigen, as the support for using preordered/natural orderings
is in the master branch but has not been released yet.

Change-Id: I25588d3e723e50606f327db5759f174f58439e29
2014-08-01 16:42:00 -07:00
Sameer Agarwal b43e73a034 Simplify the Eigen code in SparseNormalCholeskySolver.
Simplifying some of the template handling, and remove the use
of SelfAdjointView as it is not needed. The solver itself takes
an argument for where the data is actually stored.

The performance of SparseNormalCholesky with EIGEN_SPARSE
seems to be on par with CX_SPARSE.

Change-Id: I69e22a144b447c052b6cbe59ef1aa33eae2dd9e3
2014-08-01 12:10:04 -07:00
Sameer Agarwal 031598295c Enable Eigen as sparse linear algebra library.
SPARSE_NORMAL_CHOLESKY and SPARSE_SCHUR can now be used
with EIGEN_SPARSE as the backend.

The performance is not as good as CXSparse. This needs to be
investigated. Is it because the quality of AMD ordering that
we are computing is not as good as the one for CXSparse? This
could be because we are working with the scalar matrix instead
of the block matrix.

Also, the upper/lower triangular story is not completely clear.
Both of these issues will be benchmarked and tackled in the
near future.

Also included in this change is a bunch of cleanup to the
SparseNormalCholeskySolver and SparseSchurComplementSolver
classes around the use of the of defines used to conditionally
compile out parts of the code.

The system_test has been updated to test EIGEN_SPARSE also.

Change-Id: I46a57e9c4c97782696879e0b15cfc7a93fe5496a
2014-07-31 22:05:34 -07:00
Sameer Agarwal 1b17145adf Make canned loss functions more robust.
The loss functions that ship with ceres can sometimes
generate a zero first derivative if the residual is too
large.

In such cases Corrector fails with an ugly undebuggable
crash. This CL is the first in a series of fixes to
take care of this.

We clamp the values of rho' from below by
numeric_limits<double>::min().

Also included here is some minor cleanup where the constants
are treated as doubles rather than integers.

Thanks to Pierre Moulon for reporting this problem.

Change-Id: I3aaf375303ecc2659bbf6fb56a812e7dc3a41106
2014-07-30 10:14:15 -07:00
Alex Stewart b27a2044b3 Fix type of suppressed compiler warning for Eigen 3.2.0.
- Warning type generated is unused-but-set-variable, not unused-variable
  and the corresponding "local variable is initialized but not
  referenced" MSVC warning.

Change-Id: I716fd6a87cc91533bc862b2ef932601c8857818d
2014-07-22 19:38:50 +01:00
Alex Stewart b76615a670 Suppress unused variable warning from Eigen 3.2.0.
- Eigen 3.2.0 emits an unused variable warning from Ordering.h
  #included by Eigen/SparseQR which prevents Ceres compilation due
  to -Werror.
- As Eigen 3.2.0 is still in widespread use, we explicitly disable
  the warning only around the inclusion of Eigen/SparseQR.

Change-Id: I48ce905ef6a22bc1b06f9e5bed328bd315f407f0
2014-07-22 18:18:08 +00:00
Sameer Agarwal c89e82e001 Drop iostream from solver.cc
Change-Id: I512a247c183113ef5dcf09e13f19c17c83293f21
2014-07-22 10:53:32 -07:00
Sameer Agarwal bb3b865883 Add sstream header to solver.cc
Thanks to Filippo Basso for reporting this.

Change-Id: I45059cd66bad9d26e706fc3011f714cf60121320
2014-07-22 10:47:46 -07:00
Sameer Agarwal 060a850602 Remove SPARSE_CHOLESKY based covariance estimation.
Sparse Cholesky factorization is not rank revealing. Therefore
this algorithm cannot reliably tell when the Jacobian matrix is
rank deficient or so poorly conditioned that the covariance matrix
cannot be estimated.

Making things worse, this algorithm works on the normal equations,
which makes the conditioning problem much worse.

This change, deletes the SPARSE_CHOLESKY algorithm in the covariance
estimation code. Also to make the naming consistent, it renames

SPARSE_QR -> SUITE_SPARSE_QR

so that it parallels EIGEN_SPARSE_QR.

Also, since we now have EIGEN_SPARSE_QR, we can default to using
it when SuiteSparse is not available instead of DENSE_SVD, which
generally speaking should only be used by folks who are dealing
with small rank deficient jacobians.

Change-Id: I8b134c7e8a2e86ca374371f185b19f1c3e74349c
2014-07-20 07:35:35 -07:00
Mike Vitus 0bbb48a941 Adds support for computing the covariance using Eigen's sparse QR module.
For smaller problems Eigen is faster than SuiteSparseQR. This has been
tested with Eigen 3.2.1. Below are detailed timings. Problem 1 is the
smallest and problem 3 is the largest. The timings below are:
mean +- standard deviation.

Problem 1:
Eigen       0.0009218 +- 0.0002755
SuiteSparse 0.001406 +- 0.001610

Problem 2:
Eigen       0.002338 +- 0.001005
SuiteSparse 0.001910 +- 0.0004513

Problem 3:
Eigen       0.005455 +- 0.001759
SuiteSparse 0.002411 +- 0.0004974

Detailed problem descriptions:

Problem 1 size:
                               Original                  Reduced
Parameter blocks                533                       54
Parameters                      368                      104
Effective parameters           1201                       94
Residual blocks                 233                       77
Residual                       1194                      258

Problem 2 size:
                              Original                  Reduced
Parameter blocks                573                       84
Parameters                     1458                      184
Effective parameters           1281                      164
Residual blocks                 263                      107
Residual                       1314                      378

Problem 3 size:
                              Original                  Reduced
Parameter blocks                613                      114
Parameters                     1548                      264
Effective parameters           1361                      234
Residual blocks                 293                      137
Residual                       1434                      498

Change-Id: I884a67e2f728fe2992812148d82ccf5f27864fd7
2014-07-11 14:42:48 -07:00
Sameer Agarwal 603277d343 Add check for dynamic sparsity in Solver::Options::IsValid.
Dynamic sparsity should only be used with SPARSE_NORMAL_CHOLESKY.

Change-Id: I8dd33c05ec0337b3c13b547ecffe3aefc3e5ab12
2014-06-10 10:07:53 -07:00
Sameer Agarwal e9fcf3decb Add OrderedGroups::MinNonZeroGroup.
Change-Id: If571c2435a7c884b472c33421722208cb2d036ff
2014-06-09 13:14:05 -07:00
Alex Stewart ec6bfa5b12 Compile miniglog into Ceres if enabled on all platforms.
- Previously if miniglog was being used (on a non-Android system), we
  compiled it into a separate library, against which Ceres then linked.
- This was unsatisfactory as it required miniglog being built as a
  static library when building Ceres as a Windows DLL, because miniglog
  did not use the dllexport/dllimport statements, whilst for other
  platforms when building Ceres as a shared library, miniglog needed to
  be compiled as a shared library.

- We now compile miniglog into Ceres on all platforms, not just on
  Android.
- miniglog now uses the CERES_EXPORT macro to support Windows DLLs.
  This means that miniglog now depends on Ceres' internal/port.h (and
  thus internal/config.h) which define the CERES_EXPORT macro and
  control its behaviour respectively.
- miniglog now also uses localtime_s, not localtime on Windows.

Change-Id: Ia55b9af8b4e6decf067eab92f0a5c2d14358a1e9
2014-06-09 19:02:07 +00:00
Sameer Agarwal 0cc1bc5bbd Add missing files to the NDK build.
Add reorder_program.cc and also fix an include
error which was only triggered by the NDK build.

Change-Id: Ic800687d1857be1e3d60a44b87595d59e52c1117
2014-06-06 15:24:03 -07:00
Sameer Agarwal e497f4939a Refactor reordering routines.
Move all the program reordering programs into their own file.
Also split the reordering routines for SPARSE_NORMAL_CHOLESKY
into individual library dependent routines.

Also get rid of RemovedFixedBlocksFromProgram.

Change-Id: Ie969f529e6d20dded9da021b9df1a040e08287c1
2014-06-06 05:32:21 +00:00
Sameer Agarwal 14ec849eac Change logging for TRUST_REGION.
The current logging for trust region minimizer is very informative
but cryptic. Single or double letter abbreviations must be deciphered
to make sense of it.

Here is an alternate take on it.

Before:

   0: f: 4.185660e+06 d: 0.00e+00 g: 2.16e+07 h: 0.00e+00 rho: 0.00e+00 mu: 1.00e+04 li:  0 it: 7.36e-02 tt: 3.47e-01
   1: f: 1.980525e+05 d: 3.99e+06 g: 5.34e+06 h: 2.40e+03 rho: 9.60e-01 mu: 3.00e+04 li:  1 it: 1.80e-01 tt: 5.27e-01
   2: f: 5.086543e+04 d: 1.47e+05 g: 2.11e+06 h: 1.01e+03 rho: 8.22e-01 mu: 4.09e+04 li:  1 it: 1.57e-01 tt: 6.84e-01
   3: f: 1.859667e+04 d: 3.23e+04 g: 2.87e+05 h: 2.64e+02 rho: 9.85e-01 mu: 1.23e+05 li:  1 it: 1.50e-01 tt: 8.34e-01
   4: f: 1.803857e+04 d: 5.58e+02 g: 2.69e+04 h: 8.66e+01 rho: 9.93e-01 mu: 3.69e+05 li:  1 it: 1.47e-01 tt: 9.82e-01
   5: f: 1.803391e+04 d: 4.66e+00 g: 3.11e+02 h: 1.02e+01 rho: 1.00e+00 mu: 1.11e+06 li:  1 it: 1.51e-01 tt: 1.13e+00

After:

iter      cost      cost_change  |gradient|   |step|    tr_ratio  tr_radius  ls_iter  iter_time  total_time
   0  4.185660e+06    0.00e+00    2.16e+07   0.00e+00   0.00e+00  1.00e+04       0    7.16e-02    3.27e-01
   1  1.980525e+05    3.99e+06    5.34e+06   2.40e+03   9.60e-01  3.00e+04       1    1.77e-01    5.04e-01
   2  5.086543e+04    1.47e+05    2.11e+06   1.01e+03   8.22e-01  4.09e+04       1    1.49e-01    6.54e-01
   3  1.859667e+04    3.23e+04    2.87e+05   2.64e+02   9.85e-01  1.23e+05       1    1.45e-01    7.99e-01
   4  1.803857e+04    5.58e+02    2.69e+04   8.66e+01   9.93e-01  3.69e+05       1    1.46e-01    9.45e-01
   5  1.803391e+04    4.66e+00    3.11e+02   1.02e+01   1.00e+00  1.11e+06       1    1.48e-01    1.09e+00

Change-Id: I251f99d474028e07cb709965d513809c443884b6
2014-06-05 19:09:29 +00:00
Björn Piltz add50e3697 Don't build miniglog unconditionally as a static library
This is only needed on Windows because of lacking dllexport's.

Change-Id: I7ee4b3fd640d5870af2a73d7bfa6ccc8356b2fa0
2014-06-04 10:23:41 +02:00
Björn Piltz 4c5f6cd032 Added missing include.
MSVC complained about not finding std::min(),

Change-Id: If0a1eb77fa2988cabf26da4a49bf018e34a38b9c
2014-06-04 10:17:04 +02:00
Sameer Agarwal c8063df917 POLAK_RIBIRERE -> POLAK_RIBIERE
Thanks to Vladimir Chalupecky for reporting this.

Change-Id: I2e419415394f5d2be35b825d7c777b01ff31add1
2014-06-03 20:30:15 -07:00
Sameer Agarwal 70af9a58f1 Improve logging in Solver::Options::IsValid.
The error message now includes the value of the offending option.

Change-Id: Ic2493709f54f7efe59313d51d1ff9638191c8b38
2014-06-03 16:06:23 +00:00
Sameer Agarwal 79df23ed78 Remove NDK/Eigen bug work around.
The bug was multiple compilers ago. Now is as good a point as
any to remove the work around.

Change-Id: I1f14732d042fe061adcb31b14a6b39d8fc0ca69e
2014-06-02 21:33:40 -07:00
Sameer Agarwal 5d51e4d4bc Addressing comments from Jim Roseborough.
Change-Id: I9f5bdb4a81cd25ee4ea5de65aeddcaa0c5d18197
2014-06-02 08:12:11 -07:00
Sameer Agarwal b4a5f7ea25 Program::RemoveFixedBlocks -> Program::CreateReducedProgram.
CreateReducedProgram is a safer API, reduces the possibility of
memory leaks and produces valid programs.

Change-Id: I094d53d207fced970a4d9ea0b66cdb09ce5f0657
2014-06-01 16:38:08 -07:00
Sameer Agarwal 29fe072781 Solver::Options::min_relative_decrease can be zero.
Change-Id: I82ae5227ad0f8f6ff21e859e11f0d18ac7d9828c
2014-06-01 00:25:34 -07:00
Sameer Agarwal 226a95e488 Fix more typos in solver.cc
A number of typos in solver.cc which were hidden behind ifdefs
and were only revealed when building under android.

Change-Id: I89cbe8056da071479d2d1a251a8919b0be1f1cd4
2014-06-01 06:55:18 +00:00
Sameer Agarwal 7135be7b34 Conditionally disable SparseNormalCholesky.
Disable creation of SparseNormalCholesky if CXSparse and
SuiteSparse are not evailable.

Change-Id: I9a3beafef0073d1c55305beb0af31bc67ef39691
2014-05-31 23:44:18 -07:00
Sameer Agarwal e911db19aa Fix a memory leak in program_test.cc
Program::RemoveFixedBocks will happily drop all fixed parameter
and residual blocks. So if it is called on the one and only copy
of a program, like it was being called in the tests, this will
result in a memory leak because we would have lost track of the
fixed parameter and residual blocks.

This is not a problem in actual usage since CreateReducedProgram
will first create a copy of the program and then remove the fixed
blocks from it. But in the tests, we were creating a ProblemImpl
and then calling RemovedFixedBlocks on the underlying program
object causing a memory leak.

The fix to make a copy in the tests and then work on that. I
have also added a warning in program.h

Change-Id: I03a5f7a7f5453aec848451a5c0ace4b065f71e9b
2014-05-31 23:21:44 -07:00
Sameer Agarwal 19dee8d591 Fix a typo in solver.cc
Change-Id: I3f2e058e2d45c72eaa12129ff0b9366332ee87f5
2014-05-31 23:04:44 -07:00
Sameer Agarwal 99dead5cbf Move inner iteration ordering related methods.
Inner iterations require specific constraints on parameter
block orderings. The creation and validation routines for
these orderings are not static methods in CoordinateDescentMinimizer.

Change-Id: Ifc89491c9a8672e08523191b74b53058cbfa1db3
2014-05-30 16:16:27 -07:00
Sameer Agarwal 1228a4f332 Move alternate linear solver and preconditioner policy.
Move functions that determine alternatives to Schur type linear
solver and preconditioners into the LinearSolver and Preconditioner
interfaces.

Change-Id: Iae900afb7db17cdbeb7753497005a48c3144e2d7
2014-05-30 15:58:44 -07:00
Sameer Agarwal 1693645a67 Add Solver::Options::IsValid.
This provides a user visible way to validate the Solver::Options
before calling Solve.

Change-Id: Ife84fd33532ab2ccb7ac95abe22735843db51fde
2014-05-30 13:00:18 -07:00