Minor fixes
Based on William Rucklidge's review, including
a nasty bug in parameter block removal.
Change-Id: I3a692e589f600ff560ecae9fa85bb0b76063d403
diff --git a/include/ceres/rotation.h b/include/ceres/rotation.h
index 74cbcf1..711687f 100644
--- a/include/ceres/rotation.h
+++ b/include/ceres/rotation.h
@@ -51,13 +51,19 @@
namespace ceres {
-// Trivial wrapper to index linear arrays as matrices, given a fixed column and
-// row stride. When an array "T* arr" is wrapped by a
-// "(const) MatrixAdapter<T, row_stride, col_stride> M", the expression "M(i, j)" is
-// equivalent to "arr[i * row_stride + j * col_stride]".
-// Conversion functions to and from rotation matrices accept MatrixAdapters to
-// permit using row-major and column-major layouts, and rotation matrices embedded in
-// larger matrices (such as a 3x4 projection matrix).
+// Trivial wrapper to index linear arrays as matrices, given a fixed
+// column and row stride. When an array "T* array" is wrapped by a
+//
+// (const) MatrixAdapter<T, row_stride, col_stride> M"
+//
+// the expression M(i, j) is equivalent to
+//
+// arrary[i * row_stride + j * col_stride]
+//
+// Conversion functions to and from rotation matrices accept
+// MatrixAdapters to permit using row-major and column-major layouts,
+// and rotation matrices embedded in larger matrices (such as a 3x4
+// projection matrix).
template <typename T, int row_stride, int col_stride>
struct MatrixAdapter;
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) {
diff --git a/scripts/make_docs.py b/scripts/make_docs.py
index 5bffd5b..41879cb 100644
--- a/scripts/make_docs.py
+++ b/scripts/make_docs.py
@@ -38,7 +38,7 @@
if len(sys.argv) < 3:
print "make_docs.py src_root destination_root"
- sys.exit(1);
+ sys.exit(1)
src_dir = sys.argv[1] + "/docs/source"
build_root = sys.argv[2]
@@ -58,8 +58,10 @@
</script>"""
# By default MathJax uses does not use TeX fonts. This simple search
-# an replace fixes that.
+# and replace fixes that.
for name in glob.glob("%s/*.html" % html_dir):
print "Postprocessing: ", name
- out = open(name).read().replace(input_pattern, output_pattern)
+ fptr = open(name)
+ out = fptr.read().replace(input_pattern, output_pattern)
open(name, "w").write(out)
+ fptr.close();
