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
diff --git a/include/ceres/internal/numeric_diff.h b/include/ceres/internal/numeric_diff.h
index 5048348..3b264b4 100644
--- a/include/ceres/internal/numeric_diff.h
+++ b/include/ceres/internal/numeric_diff.h
@@ -103,14 +103,17 @@
typedef Matrix<double, kNumResiduals, 1> ResidualVector;
typedef Matrix<double, kParameterBlockSize, 1> ParameterVector;
+
+ // The convoluted reasoning for choosing the Row/Column major
+ // ordering of the matrix is an artifact of the restrictions in
+ // Eigen that prevent it from creating RowMajor matrices with a
+ // single column. In these cases, we ask for a ColMajor matrix.
typedef Matrix<double,
kNumResiduals,
kParameterBlockSize,
- (kParameterBlockSize == 1 &&
- kNumResiduals > 1) ? ColMajor : RowMajor>
+ (kParameterBlockSize == 1) ? ColMajor : RowMajor>
JacobianMatrix;
-
Map<JacobianMatrix> parameter_jacobian(jacobian,
NUM_RESIDUALS,
kParameterBlockSize);
diff --git a/internal/ceres/numeric_diff_cost_function_test.cc b/internal/ceres/numeric_diff_cost_function_test.cc
index 422c712..b3d8817 100644
--- a/internal/ceres/numeric_diff_cost_function_test.cc
+++ b/internal/ceres/numeric_diff_cost_function_test.cc
@@ -182,6 +182,30 @@
cost_function.reset(
new NumericDiffCostFunction<SizeTestingCostFunction<2,2>, CENTRAL, 2, 2>(
new SizeTestingCostFunction<2,2>, ceres::TAKE_OWNERSHIP));
+
+ cost_function.reset(
+ new NumericDiffCostFunction<EasyFunctor, CENTRAL, ceres::DYNAMIC, 1, 1>(
+ new EasyFunctor, TAKE_OWNERSHIP, 1));
+
+ cost_function.reset(
+ new NumericDiffCostFunction<EasyFunctor, CENTRAL, ceres::DYNAMIC, 1, 1>(
+ new EasyFunctor, TAKE_OWNERSHIP, 2));
+
+ cost_function.reset(
+ new NumericDiffCostFunction<EasyFunctor, CENTRAL, ceres::DYNAMIC, 1, 2>(
+ new EasyFunctor, TAKE_OWNERSHIP, 1));
+
+ cost_function.reset(
+ new NumericDiffCostFunction<EasyFunctor, CENTRAL, ceres::DYNAMIC, 1, 2>(
+ new EasyFunctor, TAKE_OWNERSHIP, 2));
+
+ cost_function.reset(
+ new NumericDiffCostFunction<EasyFunctor, CENTRAL, ceres::DYNAMIC, 2, 1>(
+ new EasyFunctor, TAKE_OWNERSHIP, 1));
+
+ cost_function.reset(
+ new NumericDiffCostFunction<EasyFunctor, CENTRAL, ceres::DYNAMIC, 2, 1>(
+ new EasyFunctor, TAKE_OWNERSHIP, 2));
}
TEST(NumericDiffCostFunction, EasyCaseFunctorCentralDifferencesAndDynamicNumResiduals) {