commit 4b8c731d8a4f3fda53c642ff14a25fab6c233918
author Sameer Agarwal <sameeragarwal@google.com> Fri Jul 31 10:05:52 2020 -0700
committer Sameer Agarwal <sameeragarwal@google.com> Fri Jul 31 10:05:52 2020 -0700
tree 60379b114e7d3007f6198dcf32cc15b7396574e3
parent 5cb5b35a930c1702278083c75769dbb4e5801045

Fix a bug in DynamicAutoDiffCostFunction

DynamicAutoDiffCostFunction::Evaluate when provided with a jacobians
array that was non-empty but all its entries are nullptr, would
compute num_active_parameters = 0, and then skip over all the loops
that evaluated the CostFunctor.

The fix is to check if num_active_parameters == 0, and then treat
it as the case where jacobians array is null.

Thanks to Ky Waegel for reporting and providing a reproduction for this.

Change-Id: Ib86930c2c3f722724d249f662bf88238679bbf98

include/ceres/dynamic_autodiff_cost_function.h

diff --git a/include/ceres/dynamic_autodiff_cost_function.h b/include/ceres/dynamic_autodiff_cost_function.h
index 7b75150..1027064 100644
--- a/include/ceres/dynamic_autodiff_cost_function.h
+++ b/include/ceres/dynamic_autodiff_cost_function.h
@@ -151,6 +151,9 @@
       }
     }
 
+    if (num_active_parameters == 0) {
+      return (*functor_)(parameters, residuals);
+    }
     // When `num_active_parameters % Stride != 0` then it can be the case
     // that `active_parameter_count < Stride` while parameter_cursor is less
     // than the total number of parameters and with no remaining non-constant

internal/ceres/dynamic_autodiff_cost_function_test.cc

diff --git a/internal/ceres/dynamic_autodiff_cost_function_test.cc b/internal/ceres/dynamic_autodiff_cost_function_test.cc
index bec31c5..6b57fd3 100644
--- a/internal/ceres/dynamic_autodiff_cost_function_test.cc
+++ b/internal/ceres/dynamic_autodiff_cost_function_test.cc
@@ -771,5 +771,51 @@
   }
 }
 
+class ValueError {
+ public:
+  explicit ValueError(double target_value) : target_value_(target_value) {}
+
+  template <typename T>
+  bool operator()(const T* value, T* residual) const {
+    *residual = *value - T(target_value_);
+    return true;
+  }
+
+ protected:
+  double target_value_;
+};
+
+class DynamicValueError {
+ public:
+  explicit DynamicValueError(double target_value)
+      : target_value_(target_value) {}
+
+  template <typename T>
+  bool operator()(T const* const* parameters, T* residual) const {
+    residual[0] = T(target_value_) - parameters[0][0];
+    return true;
+  }
+
+ protected:
+  double target_value_;
+};
+
+TEST(DynamicAutoDiffCostFunction,
+     EvaluateWithEmptyJacobiansArrayComputesResidual) {
+  const double target_value = 1.0;
+  double parameter = 0;
+  ceres::DynamicAutoDiffCostFunction<DynamicValueError, 1> cost_function(
+      new DynamicValueError(target_value));
+  cost_function.AddParameterBlock(1);
+  cost_function.SetNumResiduals(1);
+
+  double* parameter_blocks[1] = {&parameter};
+  double* jacobians[1] = {nullptr};
+  double residual;
+
+  EXPECT_TRUE(cost_function.Evaluate(parameter_blocks, &residual, jacobians));
+  EXPECT_EQ(residual, target_value);
+}
+
 }  // namespace internal
 }  // namespace ceres