Minor fixes
Based on William Rucklidge's review, including
a nasty bug in parameter block removal.
Change-Id: I3a692e589f600ff560ecae9fa85bb0b76063d403
diff --git a/internal/ceres/block_jacobi_preconditioner.h b/internal/ceres/block_jacobi_preconditioner.h
index 32221f5..ed5eebc 100644
--- a/internal/ceres/block_jacobi_preconditioner.h
+++ b/internal/ceres/block_jacobi_preconditioner.h
@@ -54,7 +54,7 @@
class BlockJacobiPreconditioner : public Preconditioner {
public:
// A must remain valid while the BlockJacobiPreconditioner is.
- BlockJacobiPreconditioner(const BlockSparseMatrixBase& A);
+ explicit BlockJacobiPreconditioner(const BlockSparseMatrixBase& A);
virtual ~BlockJacobiPreconditioner();
// Preconditioner interface
diff --git a/internal/ceres/iterative_schur_complement_solver.cc b/internal/ceres/iterative_schur_complement_solver.cc
index c44bd39..d352d02 100644
--- a/internal/ceres/iterative_schur_complement_solver.cc
+++ b/internal/ceres/iterative_schur_complement_solver.cc
@@ -110,12 +110,6 @@
case IDENTITY:
break;
case JACOBI:
- // We need to strip the constness of the block_diagonal_FtF_inverse
- // matrix here because the only other way to initialize the struct
- // cg_solve_options would be to add a constructor to it. We know
- // that the only method ever called on the preconditioner is the
- // RightMultiply which is a const method so we don't need to worry
- // about the object getting modified.
preconditioner_.reset(
new SparseMatrixPreconditionerWrapper(
schur_complement_->block_diagonal_FtF_inverse()));
diff --git a/internal/ceres/parameter_block.h b/internal/ceres/parameter_block.h
index 3a709f4..b1e8d93 100644
--- a/internal/ceres/parameter_block.h
+++ b/internal/ceres/parameter_block.h
@@ -73,7 +73,7 @@
// Create a parameter block with the user state, size, and index specified.
// The size is the size of the parameter block and the index is the position
- // if the parameter block inside a Program (if any).
+ // of the parameter block inside a Program (if any).
ParameterBlock(double* user_state, int size, int index) {
Init(user_state, size, index, NULL);
}
diff --git a/internal/ceres/preconditioner.h b/internal/ceres/preconditioner.h
index 77280c3..5bb077e 100644
--- a/internal/ceres/preconditioner.h
+++ b/internal/ceres/preconditioner.h
@@ -69,11 +69,11 @@
//
// For example if elimination_groups is a vector of size k, then
// the linear solver is informed that it should eliminate the
- // parameter blocks 0 - elimination_groups[0] - 1 first, and then
- // elimination_groups[0] - elimination_groups[1] and so on. Within
- // each elimination group, the linear solver is free to choose how
- // the parameter blocks are ordered. Different linear solvers have
- // differing requirements on elimination_groups.
+ // parameter blocks 0 ... elimination_groups[0] - 1 first, and
+ // then elimination_groups[0] ... elimination_groups[1] and so
+ // on. Within each elimination group, the linear solver is free to
+ // choose how the parameter blocks are ordered. Different linear
+ // solvers have differing requirements on elimination_groups.
//
// The most common use is for Schur type solvers, where there
// should be at least two elimination groups and the first
@@ -130,7 +130,7 @@
class SparseMatrixPreconditionerWrapper : public Preconditioner {
public:
// Wrapper does NOT take ownership of the matrix pointer.
- SparseMatrixPreconditionerWrapper(const SparseMatrix* matrix);
+ explicit SparseMatrixPreconditionerWrapper(const SparseMatrix* matrix);
virtual ~SparseMatrixPreconditionerWrapper();
// Preconditioner interface
diff --git a/internal/ceres/problem_impl.cc b/internal/ceres/problem_impl.cc
index 6154ddf..ae4e7ed 100644
--- a/internal/ceres/problem_impl.cc
+++ b/internal/ceres/problem_impl.cc
@@ -177,7 +177,7 @@
ProblemImpl::~ProblemImpl() {
// Collect the unique cost/loss functions and delete the residuals.
- const int num_residual_blocks = program_->residual_blocks_.size();
+ const int num_residual_blocks = program_->residual_blocks_.size();
cost_functions_to_delete_.reserve(num_residual_blocks);
loss_functions_to_delete_.reserve(num_residual_blocks);
for (int i = 0; i < program_->residual_blocks_.size(); ++i) {
@@ -486,8 +486,8 @@
ResidualBlock* residual_block =
(*(program_->mutable_residual_blocks()))[i];
const int num_parameter_blocks = residual_block->NumParameterBlocks();
- for (int i = 0; i < num_parameter_blocks; ++i) {
- if (residual_block->parameter_blocks()[i] == parameter_block) {
+ for (int j = 0; j < num_parameter_blocks; ++j) {
+ if (residual_block->parameter_blocks()[j] == parameter_block) {
RemoveResidualBlock(residual_block);
// The parameter blocks are guaranteed unique.
break;
diff --git a/internal/ceres/solver.cc b/internal/ceres/solver.cc
index 6d38535..b1bd2d8 100644
--- a/internal/ceres/solver.cc
+++ b/internal/ceres/solver.cc
@@ -311,7 +311,7 @@
StringAppendF(&report, "\n");
- StringAppendF(&report, "%45s %21s\n", "Given", "Used");
+ StringAppendF(&report, "%45s %21s\n", "Given", "Used");
StringAppendF(&report, "Threads: % 25d% 25d\n",
num_threads_given, num_threads_used);
diff --git a/internal/ceres/stl_util.h b/internal/ceres/stl_util.h
index 5e92eff..b23e7c1 100644
--- a/internal/ceres/stl_util.h
+++ b/internal/ceres/stl_util.h
@@ -53,6 +53,8 @@
}
}
+// Variant of STLDeleteContainerPointers which allows the container to
+// contain duplicates.
template <class ForwardIterator>
void STLDeleteUniqueContainerPointers(ForwardIterator begin,
ForwardIterator end) {
