Fix a MSVC type deduction bug in ComputeHouseholderVector
A recent change made this function templated and MSVC 16 has trouble
doing automatic argument deduction, so the type of the template is
simplified and all callsites are explicitly annotated with the type
of the arguments.
Change-Id: I83cd0269e6e82c4a8f4e391f5fc03b92c942f74d
diff --git a/include/ceres/internal/householder_vector.h b/include/ceres/internal/householder_vector.h
index 051a88d..55f68e5 100644
--- a/include/ceres/internal/householder_vector.h
+++ b/include/ceres/internal/householder_vector.h
@@ -42,8 +42,11 @@
// vector as pivot instead of first. This computes the vector v with v(n) = 1
// and beta such that H = I - beta * v * v^T is orthogonal and
// H * x = ||x||_2 * e_n.
-template <typename Derived, typename Scalar, int N>
-void ComputeHouseholderVector(const Eigen::DenseBase<Derived>& x,
+//
+// NOTE: Some versions of MSVC have trouble deducing the type of v if
+// you do not specify all the template arguments explicitly.
+template <typename XVectorType, typename Scalar, int N>
+void ComputeHouseholderVector(const XVectorType& x,
Eigen::Matrix<Scalar, N, 1>* v,
Scalar* beta) {
CHECK(beta != nullptr);
diff --git a/include/ceres/internal/line_parameterization.h b/include/ceres/internal/line_parameterization.h
index b2ec9e1..eda3901 100644
--- a/include/ceres/internal/line_parameterization.h
+++ b/include/ceres/internal/line_parameterization.h
@@ -53,8 +53,8 @@
// d* = Plus_d(d, delta_d)
// o* = Plus_o(o, d, delta_o)
//
- // The direction update function Plus_d is the same as for the homogeneous vector
- // parameterization:
+ // The direction update function Plus_d is the same as for the homogeneous
+ // vector parameterization:
//
// d* = H_{v(d)} [0.5 sinc(0.5 |delta_d|) delta_d, cos(0.5 |delta_d|)]^T
//
@@ -70,11 +70,11 @@
static constexpr int kDim = AmbientSpaceDimension;
using AmbientVector = Eigen::Matrix<double, kDim, 1>;
using AmbientVectorRef = Eigen::Map<Eigen::Matrix<double, kDim, 1>>;
- using ConstAmbientVectorRef = Eigen::Map<const Eigen::Matrix<double, kDim, 1>>;
+ using ConstAmbientVectorRef =
+ Eigen::Map<const Eigen::Matrix<double, kDim, 1>>;
using ConstTangentVectorRef =
Eigen::Map<const Eigen::Matrix<double, kDim - 1, 1>>;
-
-
+
ConstAmbientVectorRef o(x_ptr);
ConstAmbientVectorRef d(x_ptr + kDim);
@@ -99,7 +99,12 @@
// Calculate the householder transformation which is needed for f_d and f_o.
AmbientVector v;
double beta;
- internal::ComputeHouseholderVector(d, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ internal::ComputeHouseholderVector<ConstAmbientVectorRef, double, kDim>(
+ d, &v, &beta);
if (norm_delta_d != 0.0) {
// Map the delta from the minimum representation to the over parameterized
@@ -139,7 +144,8 @@
const double* x_ptr, double* jacobian_ptr) const {
static constexpr int kDim = AmbientSpaceDimension;
using AmbientVector = Eigen::Matrix<double, kDim, 1>;
- using ConstAmbientVectorRef = Eigen::Map<const Eigen::Matrix<double, kDim, 1>>;
+ using ConstAmbientVectorRef =
+ Eigen::Map<const Eigen::Matrix<double, kDim, 1>>;
using MatrixRef = Eigen::Map<
Eigen::Matrix<double, 2 * kDim, 2 * (kDim - 1), Eigen::RowMajor>>;
@@ -151,7 +157,12 @@
AmbientVector v;
double beta;
- internal::ComputeHouseholderVector(d, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ internal::ComputeHouseholderVector<ConstAmbientVectorRef, double, kDim>(
+ d, &v, &beta);
// The Jacobian is equal to J = 0.5 * H.leftCols(kDim - 1) where H is
// the Householder matrix (H = I - beta * v * v') for the origin point. For
diff --git a/internal/ceres/householder_vector_test.cc b/internal/ceres/householder_vector_test.cc
index 9eaca75..10b3e88 100644
--- a/internal/ceres/householder_vector_test.cc
+++ b/internal/ceres/householder_vector_test.cc
@@ -42,7 +42,11 @@
// Check to ensure that H * x = ||x|| * [0 ... 0 1]'.
Vector v(x.rows());
double beta;
- ComputeHouseholderVector(x, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ ComputeHouseholderVector<Vector, double, Eigen::Dynamic>(x, &v, &beta);
Vector result = x - beta * v * (v.transpose() * x);
Vector expected_result(x.rows());
diff --git a/internal/ceres/local_parameterization.cc b/internal/ceres/local_parameterization.cc
index 5fedefa..97fdbed 100644
--- a/internal/ceres/local_parameterization.cc
+++ b/internal/ceres/local_parameterization.cc
@@ -31,10 +31,11 @@
#include "ceres/local_parameterization.h"
#include <algorithm>
+
#include "Eigen/Geometry"
-#include "ceres/internal/householder_vector.h"
#include "ceres/internal/eigen.h"
#include "ceres/internal/fixed_array.h"
+#include "ceres/internal/householder_vector.h"
#include "ceres/rotation.h"
#include "glog/logging.h"
@@ -42,8 +43,7 @@
using std::vector;
-LocalParameterization::~LocalParameterization() {
-}
+LocalParameterization::~LocalParameterization() {}
bool LocalParameterization::MultiplyByJacobian(const double* x,
const int num_rows,
@@ -86,9 +86,8 @@
const int num_cols,
const double* global_matrix,
double* local_matrix) const {
- std::copy(global_matrix,
- global_matrix + num_cols * GlobalSize(),
- local_matrix);
+ std::copy(
+ global_matrix, global_matrix + num_cols * GlobalSize(), local_matrix);
return true;
}
@@ -183,10 +182,12 @@
bool QuaternionParameterization::ComputeJacobian(const double* x,
double* jacobian) const {
- jacobian[0] = -x[1]; jacobian[1] = -x[2]; jacobian[2] = -x[3]; // NOLINT
- jacobian[3] = x[0]; jacobian[4] = x[3]; jacobian[5] = -x[2]; // NOLINT
- jacobian[6] = -x[3]; jacobian[7] = x[0]; jacobian[8] = x[1]; // NOLINT
- jacobian[9] = x[2]; jacobian[10] = -x[1]; jacobian[11] = x[0]; // NOLINT
+ // clang-format off
+ jacobian[0] = -x[1]; jacobian[1] = -x[2]; jacobian[2] = -x[3];
+ jacobian[3] = x[0]; jacobian[4] = x[3]; jacobian[5] = -x[2];
+ jacobian[6] = -x[3]; jacobian[7] = x[0]; jacobian[8] = x[1];
+ jacobian[9] = x[2]; jacobian[10] = -x[1]; jacobian[11] = x[0];
+ // clang-format on
return true;
}
@@ -216,10 +217,12 @@
bool EigenQuaternionParameterization::ComputeJacobian(const double* x,
double* jacobian) const {
- jacobian[0] = x[3]; jacobian[1] = x[2]; jacobian[2] = -x[1]; // NOLINT
- jacobian[3] = -x[2]; jacobian[4] = x[3]; jacobian[5] = x[0]; // NOLINT
- jacobian[6] = x[1]; jacobian[7] = -x[0]; jacobian[8] = x[3]; // NOLINT
- jacobian[9] = -x[0]; jacobian[10] = -x[1]; jacobian[11] = -x[2]; // NOLINT
+ // clang-format off
+ jacobian[0] = x[3]; jacobian[1] = x[2]; jacobian[2] = -x[1];
+ jacobian[3] = -x[2]; jacobian[4] = x[3]; jacobian[5] = x[0];
+ jacobian[6] = x[1]; jacobian[7] = -x[0]; jacobian[8] = x[3];
+ jacobian[9] = -x[0]; jacobian[10] = -x[1]; jacobian[11] = -x[2];
+ // clang-format on
return true;
}
@@ -248,8 +251,8 @@
// (2nd Edition) for a detailed description. Note there is a typo on Page
// 625, line 4 so check the book errata.
const double norm_delta_div_2 = 0.5 * norm_delta;
- const double sin_delta_by_delta = std::sin(norm_delta_div_2) /
- norm_delta_div_2;
+ const double sin_delta_by_delta =
+ std::sin(norm_delta_div_2) / norm_delta_div_2;
Vector y(size_);
y.head(size_ - 1) = 0.5 * sin_delta_by_delta * delta;
@@ -257,12 +260,17 @@
Vector v(size_);
double beta;
- internal::ComputeHouseholderVector(x, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ internal::ComputeHouseholderVector<ConstVectorRef, double, Eigen::Dynamic>(
+ x, &v, &beta);
// Apply the delta update to remain on the unit sphere. See section A6.9.3
// on page 625 of Hartley & Zisserman (2nd Edition) for a detailed
// description.
- x_plus_delta = x.norm() * (y - v * (beta * (v.transpose() * y)));
+ x_plus_delta = x.norm() * (y - v * (beta * (v.transpose() * y)));
return true;
}
@@ -274,7 +282,12 @@
Vector v(size_);
double beta;
- internal::ComputeHouseholderVector(x, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ internal::ComputeHouseholderVector<ConstVectorRef, double, Eigen::Dynamic>(
+ x, &v, &beta);
// The Jacobian is equal to J = 0.5 * H.leftCols(size_ - 1) where H is the
// Householder matrix (H = I - beta * v * v').
@@ -293,9 +306,8 @@
int x_cursor = 0;
int delta_cursor = 0;
for (const auto& param : local_params_) {
- if (!param->Plus(x + x_cursor,
- delta + delta_cursor,
- x_plus_delta + x_cursor)) {
+ if (!param->Plus(
+ x + x_cursor, delta + delta_cursor, x_plus_delta + x_cursor)) {
return false;
}
delta_cursor += param->LocalSize();
@@ -320,8 +332,8 @@
if (!param->ComputeJacobian(x + x_cursor, buffer.data())) {
return false;
}
- jacobian.block(x_cursor, delta_cursor, global_size, local_size)
- = MatrixRef(buffer.data(), global_size, local_size);
+ jacobian.block(x_cursor, delta_cursor, global_size, local_size) =
+ MatrixRef(buffer.data(), global_size, local_size);
delta_cursor += local_size;
x_cursor += global_size;
diff --git a/internal/ceres/local_parameterization_test.cc b/internal/ceres/local_parameterization_test.cc
index f165049..636cac2 100644
--- a/internal/ceres/local_parameterization_test.cc
+++ b/internal/ceres/local_parameterization_test.cc
@@ -452,7 +452,14 @@
Eigen::Matrix<Scalar, Dim, 1> v;
Scalar beta;
- internal::ComputeHouseholderVector(x, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ internal::ComputeHouseholderVector<
+ Eigen::Map<const Eigen::Matrix<Scalar, Dim, 1>>,
+ Scalar,
+ Dim>(x, &v, &beta);
x_plus_delta = x.norm() * (y - v * (beta * v.dot(y)));
@@ -586,7 +593,14 @@
Eigen::Matrix<Scalar, AmbientSpaceDim, 1> v;
Scalar beta;
- internal::ComputeHouseholderVector(dir, &v, &beta);
+
+ // NOTE: The explicit template arguments are needed here because
+ // ComputeHouseholderVector is templated and some versions of MSVC
+ // have trouble deducing the type of v automatically.
+ internal::ComputeHouseholderVector<
+ Eigen::Map<const Eigen::Matrix<Scalar, AmbientSpaceDim, 1>>,
+ Scalar,
+ AmbientSpaceDim>(dir, &v, &beta);
Eigen::Matrix<Scalar, AmbientSpaceDim, 1> y;
y << 0.5 * delta_origin_point, Scalar(0.0);
