Ensure that partial evaluation of residuals triggers an error
ResidualBlock evaluation has logic to ensure that CostFunction
should always fill out the residual and jacobian arrays completely
by using a special value to pre-populate these arrays.
This works for CostFunctions with analytical Jacobians but not for
AutoDiffCostFunction and NumericDiffCostFunction Jacobians.
There is no way to fix this for NumericDiffCostFunctions without
introducing significant performance penalties but the residual
evaluation fails, which should be enough to catch such errors.
For AutoDiffCostFunction the way the Jets are default initialized
was sidestepping this check. So now, the Jet that is used to
capture the output residuals is now initialized with
kImpossibleValue, which will ensure that if the user forgets
to fill all output fields, it triggers an evaluation error.
This change required that ceres::internal::kImpossibleValue be moved
out of array_utils.h/cc to types.h.
Change-Id: I35bb0946cf0785a5d43c7b5459a2272848fb2a9b
diff --git a/internal/ceres/array_utils.cc b/internal/ceres/array_utils.cc
index 7be3c78..a165a1b 100644
--- a/internal/ceres/array_utils.cc
+++ b/internal/ceres/array_utils.cc
@@ -37,19 +37,12 @@
#include <vector>
#include "ceres/fpclassify.h"
#include "ceres/stringprintf.h"
-
+#include "ceres/types.h"
namespace ceres {
namespace internal {
using std::string;
-// It is a near impossibility that user code generates this exact
-// value in normal operation, thus we will use it to fill arrays
-// before passing them to user code. If on return an element of the
-// array still contains this value, we will assume that the user code
-// did not write to that memory location.
-const double kImpossibleValue = 1e302;
-
bool IsArrayValid(const int size, const double* x) {
if (x != NULL) {
for (int i = 0; i < size; ++i) {
diff --git a/internal/ceres/array_utils.h b/internal/ceres/array_utils.h
index 2d2ffca..baf112e 100644
--- a/internal/ceres/array_utils.h
+++ b/internal/ceres/array_utils.h
@@ -66,8 +66,6 @@
// array pointer is NULL, it is treated as an array of zeros.
void AppendArrayToString(const int size, const double* x, std::string* result);
-extern const double kImpossibleValue;
-
// This routine takes an array of integer values, sorts and uniques
// them and then maps each value in the array to its position in the
// sorted+uniqued array. By doing this, if there are are k unique
diff --git a/internal/ceres/autodiff_cost_function_test.cc b/internal/ceres/autodiff_cost_function_test.cc
index 241fa0f..d14fb82 100644
--- a/internal/ceres/autodiff_cost_function_test.cc
+++ b/internal/ceres/autodiff_cost_function_test.cc
@@ -34,6 +34,7 @@
#include "gtest/gtest.h"
#include "ceres/cost_function.h"
+#include "ceres/array_utils.h"
namespace ceres {
namespace internal {
@@ -142,5 +143,30 @@
delete cost_function;
}
+struct OnlyFillsOneOutputFunctor {
+ template <typename T>
+ bool operator()(const T* x, T* output) const {
+ output[0] = x[0];
+ return true;
+ }
+};
+
+TEST(AutoDiffCostFunction, PartiallyFilledResidualShouldFailEvaluation) {
+ double parameter = 1.0;
+ double jacobian[2];
+ double residuals[2];
+ double* parameters[] = {¶meter};
+ double* jacobians[] = {jacobian};
+
+ scoped_ptr<CostFunction> cost_function(
+ new AutoDiffCostFunction<OnlyFillsOneOutputFunctor, 2, 1>(
+ new OnlyFillsOneOutputFunctor));
+ InvalidateArray(2, jacobian);
+ InvalidateArray(2, residuals);
+ EXPECT_TRUE(cost_function->Evaluate(parameters, residuals, jacobians));
+ EXPECT_FALSE(IsArrayValid(2, jacobian));
+ EXPECT_FALSE(IsArrayValid(2, residuals));
+}
+
} // namespace internal
} // namespace ceres
diff --git a/internal/ceres/numeric_diff_cost_function_test.cc b/internal/ceres/numeric_diff_cost_function_test.cc
index 13ab106..983f11e 100644
--- a/internal/ceres/numeric_diff_cost_function_test.cc
+++ b/internal/ceres/numeric_diff_cost_function_test.cc
@@ -37,6 +37,7 @@
#include <vector>
#include "ceres/internal/macros.h"
#include "ceres/internal/scoped_ptr.h"
+#include "ceres/array_utils.h"
#include "ceres/numeric_diff_test_utils.h"
#include "ceres/test_util.h"
#include "ceres/types.h"
@@ -353,6 +354,36 @@
functor.ExpectCostFunctionEvaluationIsNearlyCorrect(*cost_function);
}
+struct OnlyFillsOneOutputFunctor {
+ bool operator()(const double* x, double* output) const {
+ output[0] = x[0];
+ return true;
+ }
+};
+
+TEST(NumericDiffCostFunction, PartiallyFilledResidualShouldFailEvaluation) {
+ double parameter = 1.0;
+ double jacobian[2];
+ double residuals[2];
+ double* parameters[] = {¶meter};
+ double* jacobians[] = {jacobian};
+
+ scoped_ptr<CostFunction> cost_function(
+ new NumericDiffCostFunction<OnlyFillsOneOutputFunctor, CENTRAL, 2, 1>(
+ new OnlyFillsOneOutputFunctor));
+ InvalidateArray(2, jacobian);
+ InvalidateArray(2, residuals);
+ EXPECT_TRUE(cost_function->Evaluate(parameters, residuals, jacobians));
+ EXPECT_FALSE(IsArrayValid(2, residuals));
+ InvalidateArray(2, residuals);
+ EXPECT_TRUE(cost_function->Evaluate(parameters, residuals, NULL));
+ // We are only testing residuals here, because the Jacobians are
+ // computed using finite differencing from the residuals, so unless
+ // we introduce a validation step after every evaluation of
+ // residuals inside NumericDiffCostFunction, there is no way of
+ // ensuring that the Jacobian array is invalid.
+ EXPECT_FALSE(IsArrayValid(2, residuals));
+}
} // namespace internal
} // namespace ceres
