diff --git a/include/ceres/internal/autodiff.h b/include/ceres/internal/autodiff.h index 136152a36..bf5fb774e 100644 --- a/include/ceres/internal/autodiff.h +++ b/include/ceres/internal/autodiff.h @@ -259,6 +259,13 @@ struct AutoDiff { JetT* output = x.get() + N0 + N1 + N2 + N3 + N4 + N5 + N6 + N7 + N8 + N9; + // Invalidate the output Jets, so that we can detect if the user + // did not assign values to all of them. + for (int i = 0; i < num_outputs; ++i) { + output[i].a = kImpossibleValue; + output[i].v.setConstant(kImpossibleValue); + } + #define CERES_MAKE_1ST_ORDER_PERTURBATION(i) \ if (N ## i) { \ internal::Make1stOrderPerturbation( \ diff --git a/include/ceres/types.h b/include/ceres/types.h index 2ea418036..6b9da8498 100644 --- a/include/ceres/types.h +++ b/include/ceres/types.h @@ -424,6 +424,13 @@ enum CovarianceAlgorithmType { EIGEN_SPARSE_QR }; +// 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; + CERES_EXPORT const char* LinearSolverTypeToString( LinearSolverType type); CERES_EXPORT bool StringToLinearSolverType(std::string value, diff --git a/internal/ceres/array_utils.cc b/internal/ceres/array_utils.cc index 7be3c78ce..a165a1bdf 100644 --- a/internal/ceres/array_utils.cc +++ b/internal/ceres/array_utils.cc @@ -37,19 +37,12 @@ #include #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 2d2ffca88..baf112e71 100644 --- a/internal/ceres/array_utils.h +++ b/internal/ceres/array_utils.h @@ -66,8 +66,6 @@ int FindInvalidValue(const int size, const double* x); // 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 241fa0f67..d14fb8289 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 @@ TEST(AutodiffCostFunction, ManyParameterAutodiffInstantiates) { delete cost_function; } +struct OnlyFillsOneOutputFunctor { + template + 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 cost_function( + new AutoDiffCostFunction( + 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 13ab1069c..983f11e11 100644 --- a/internal/ceres/numeric_diff_cost_function_test.cc +++ b/internal/ceres/numeric_diff_cost_function_test.cc @@ -37,6 +37,7 @@ #include #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 @@ TEST(NumericDiffCostFunction, RandomizedCostFunctionRidders) { 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 cost_function( + new NumericDiffCostFunction( + 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