Make ConditionedCostFunction compatible with repeated CostFunction.

If the user uses the same conditioner twice, it does not lead
to a double free errors.

https://github.com/ceres-solver/ceres-solver/issues/422

Change-Id: I9041ddcbffa8dcb882a63bddb82b384897efc970
This commit is contained in:
Sameer Agarwal
2018-09-21 10:21:41 -07:00
parent 956fd1aca7
commit c65cdd7074
3 changed files with 24 additions and 7 deletions
@@ -77,6 +77,8 @@ class CERES_EXPORT ConditionedCostFunction : public CostFunction {
// per-residual conditioner. Takes ownership of all of the wrapped cost
// functions, or not, depending on the ownership parameter. Conditioners
// may be NULL, in which case the corresponding residual is not modified.
//
// The conditioners can repeat.
ConditionedCostFunction(CostFunction* wrapped_cost_function,
const std::vector<CostFunction*>& conditioners,
Ownership ownership);
+1 -1
View File
@@ -68,7 +68,7 @@ ConditionedCostFunction::ConditionedCostFunction(
ConditionedCostFunction::~ConditionedCostFunction() {
if (ownership_ == TAKE_OWNERSHIP) {
STLDeleteElements(&conditioners_);
STLDeleteUniqueContainerPointers(conditioners_.begin(), conditioners_.end());
} else {
wrapped_cost_function_.release();
}
@@ -67,7 +67,7 @@ class LinearCostFunction : public CostFunction {
};
// Tests that ConditionedCostFunction does what it's supposed to.
TEST(CostFunctionTest, ConditionedCostFunction) {
TEST(ConditionedCostFunction, NormalOperation) {
double v1[kTestCostFunctionSize], v2[kTestCostFunctionSize],
jac[kTestCostFunctionSize * kTestCostFunctionSize],
result[kTestCostFunctionSize];
@@ -92,17 +92,16 @@ TEST(CostFunctionTest, ConditionedCostFunction) {
conditioners.push_back(new LinearCostFunction(i + 2, i * 7));
}
ConditionedCostFunction conditioned_cost_function(difference_cost_function,
conditioners,
TAKE_OWNERSHIP);
ConditionedCostFunction conditioned_cost_function(
difference_cost_function, conditioners, TAKE_OWNERSHIP);
EXPECT_EQ(difference_cost_function->num_residuals(),
conditioned_cost_function.num_residuals());
EXPECT_EQ(difference_cost_function->parameter_block_sizes(),
conditioned_cost_function.parameter_block_sizes());
double *parameters[1];
double* parameters[1];
parameters[0] = v1;
double *jacs[1];
double* jacs[1];
jacs[0] = jac;
conditioned_cost_function.Evaluate(parameters, result, jacs);
@@ -122,5 +121,21 @@ TEST(CostFunctionTest, ConditionedCostFunction) {
}
}
TEST(ConditionedCostFunction, SharedConditionersDoNotTriggerDoubleFree) {
// Make a cost function that computes x - v2
double v2[kTestCostFunctionSize];
VectorRef v2_vector(v2, kTestCostFunctionSize, 1);
Matrix identity = Matrix::Identity(kTestCostFunctionSize, kTestCostFunctionSize);
NormalPrior* difference_cost_function = new NormalPrior(identity, v2_vector);
CostFunction* conditioner = new LinearCostFunction(2, 7);
std::vector<CostFunction*> conditioners;
for (int i = 0; i < kTestCostFunctionSize; i++) {
conditioners.push_back(conditioner);
}
ConditionedCostFunction conditioned_cost_function(
difference_cost_function, conditioners, TAKE_OWNERSHIP);
}
} // namespace internal
} // namespace ceres