From 12eb389b4ec4113a2260c1a192a1d3f8d1b6a2d3 Mon Sep 17 00:00:00 2001
From: Sameer Agarwal <sameeragarwal@google.com>
Date: Wed, 27 Aug 2014 22:18:33 -0700
Subject: [PATCH] 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
---
 include/ceres/internal/numeric_diff.h         |  9 ++++---
 .../ceres/numeric_diff_cost_function_test.cc  | 24 +++++++++++++++++++
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/include/ceres/internal/numeric_diff.h b/include/ceres/internal/numeric_diff.h
index 504834856..3b264b45a 100644
--- a/include/ceres/internal/numeric_diff.h
+++ b/include/ceres/internal/numeric_diff.h
@@ -103,14 +103,17 @@ struct NumericDiff {
 
     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 422c712ab..b3d8817ef 100644
--- a/internal/ceres/numeric_diff_cost_function_test.cc
+++ b/internal/ceres/numeric_diff_cost_function_test.cc
@@ -182,6 +182,30 @@ TEST(NumericDiffCostFunction, EigenRowMajorColMajorTest) {
   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) {