Program::RemoveFixedBlocks -> Program::CreateReducedProgram. CreateReducedProgram is a safer API, reduces the possibility of memory leaks and produces valid programs. Change-Id: I094d53d207fced970a4d9ea0b66cdb09ce5f0657
diff --git a/internal/ceres/program.cc b/internal/ceres/program.cc index 3a88b13..1d0a157 100644 --- a/internal/ceres/program.cc +++ b/internal/ceres/program.cc
@@ -261,6 +261,24 @@ return true; } +Program* Program::CreateReducedProgram(vector<double*>* removed_parameter_blocks, + double* fixed_cost, + string* error) const { + CHECK_NOTNULL(removed_parameter_blocks); + CHECK_NOTNULL(fixed_cost); + CHECK_NOTNULL(error); + + scoped_ptr<Program> reduced_program(new Program(*this)); + if (!reduced_program->RemoveFixedBlocks(removed_parameter_blocks, + fixed_cost, + error)) { + return NULL; + } + + reduced_program->SetParameterOffsetsAndIndex(); + return reduced_program.release(); +} + bool Program::RemoveFixedBlocks(vector<double*>* removed_parameter_blocks, double* fixed_cost, string* error) {
diff --git a/internal/ceres/program.h b/internal/ceres/program.h index 6b9190d..c7b22c4 100644 --- a/internal/ceres/program.h +++ b/internal/ceres/program.h
@@ -129,16 +129,22 @@ // Caller owns the result. TripletSparseMatrix* CreateJacobianBlockSparsityTranspose() const; - // Removes constant parameter blocks and residual blocks with no - // varying parameter blocks while preserving order. + // Create a copy of this program and removes constant parameter + // blocks and residual blocks with no varying parameter blocks while + // preserving their relative order. // - // WARNING: It is the caller's responsibility to track ownership of - // the parameter and residual blocks removed from the - // program. Generally speaking this method should only be called on - // a copy of an existing program. - bool RemoveFixedBlocks(vector<double*>* removed_parameter_blocks, - double* fixed_cost, - string* message); + // removed_parameter_blocks on exit will contain the list of + // parameter blocks that were removed. + // + // fixed_cost will be equal to the sum of the costs of the residual + // blocks that were removed. + // + // If there was a problem, then the function will return a NULL + // pointer and error will contain a human readable description of + // the problem. + Program* CreateReducedProgram(vector<double*>* removed_parameter_blocks, + double* fixed_cost, + string* error) const; // See problem.h for what these do. int NumParameterBlocks() const; @@ -157,6 +163,21 @@ string ToString() const; private: + // Remove constant parameter blocks and residual blocks with no + // varying parameter blocks while preserving their relative order. + // + // removed_parameter_blocks on exit will contain the list of + // parameter blocks that were removed. + // + // fixed_cost will be equal to the sum of the costs of the residual + // blocks that were removed. + // + // If there was a problem, then the function will return false and + // error will contain a human readable description of the problem. + bool RemoveFixedBlocks(vector<double*>* removed_parameter_blocks, + double* fixed_cost, + string* message); + // The Program does not own the ParameterBlock or ResidualBlock objects. vector<ParameterBlock*> parameter_blocks_; vector<ResidualBlock*> residual_blocks_;
diff --git a/internal/ceres/program_test.cc b/internal/ceres/program_test.cc index f273872..10bfa12 100644 --- a/internal/ceres/program_test.cc +++ b/internal/ceres/program_test.cc
@@ -88,15 +88,18 @@ problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y); problem.AddResidualBlock(new TernaryCostFunction(), NULL, &x, &y, &z); - Program program(problem.program()); vector<double*> removed_parameter_blocks; double fixed_cost = 0.0; string message; - EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks, - &fixed_cost, - &message)); - EXPECT_EQ(program.NumParameterBlocks(), 3); - EXPECT_EQ(program.NumResidualBlocks(), 3); + scoped_ptr<Program> reduced_program( + CHECK_NOTNULL(problem + .program() + .CreateReducedProgram(&removed_parameter_blocks, + &fixed_cost, + &message))); + + EXPECT_EQ(reduced_program->NumParameterBlocks(), 3); + EXPECT_EQ(reduced_program->NumResidualBlocks(), 3); EXPECT_EQ(removed_parameter_blocks.size(), 0); EXPECT_EQ(fixed_cost, 0.0); } @@ -109,15 +112,17 @@ problem.AddResidualBlock(new UnaryCostFunction(), NULL, &x); problem.SetParameterBlockConstant(&x); - Program program(problem.program()); vector<double*> removed_parameter_blocks; double fixed_cost = 0.0; string message; - EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks, - &fixed_cost, - &message)); - EXPECT_EQ(program.NumParameterBlocks(), 0); - EXPECT_EQ(program.NumResidualBlocks(), 0); + scoped_ptr<Program> reduced_program( + CHECK_NOTNULL(problem + .program() + .CreateReducedProgram(&removed_parameter_blocks, + &fixed_cost, + &message))); + EXPECT_EQ(reduced_program->NumParameterBlocks(), 0); + EXPECT_EQ(reduced_program->NumResidualBlocks(), 0); EXPECT_EQ(removed_parameter_blocks.size(), 1); EXPECT_EQ(removed_parameter_blocks[0], &x); EXPECT_EQ(fixed_cost, 9.0); @@ -134,15 +139,17 @@ problem.AddParameterBlock(&y, 1); problem.AddParameterBlock(&z, 1); - Program program(problem.program()); vector<double*> removed_parameter_blocks; double fixed_cost = 0.0; string message; - EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks, - &fixed_cost, - &message)); - EXPECT_EQ(program.NumParameterBlocks(), 0); - EXPECT_EQ(program.NumResidualBlocks(), 0); + scoped_ptr<Program> reduced_program( + CHECK_NOTNULL(problem + .program() + .CreateReducedProgram(&removed_parameter_blocks, + &fixed_cost, + &message))); + EXPECT_EQ(reduced_program->NumParameterBlocks(), 0); + EXPECT_EQ(reduced_program->NumResidualBlocks(), 0); EXPECT_EQ(removed_parameter_blocks.size(), 3); EXPECT_EQ(fixed_cost, 0.0); } @@ -161,15 +168,17 @@ problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y); problem.SetParameterBlockConstant(&x); - Program program(problem.program()); vector<double*> removed_parameter_blocks; double fixed_cost = 0.0; string message; - EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks, - &fixed_cost, - &message)); - EXPECT_EQ(program.NumParameterBlocks(), 1); - EXPECT_EQ(program.NumResidualBlocks(), 1); + scoped_ptr<Program> reduced_program( + CHECK_NOTNULL(problem + .program() + .CreateReducedProgram(&removed_parameter_blocks, + &fixed_cost, + &message))); + EXPECT_EQ(reduced_program->NumParameterBlocks(), 1); + EXPECT_EQ(reduced_program->NumResidualBlocks(), 1); } TEST(Program, RemoveFixedBlocksNumEliminateBlocks) { @@ -186,15 +195,17 @@ problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y); problem.SetParameterBlockConstant(&x); - Program program(problem.program()); vector<double*> removed_parameter_blocks; double fixed_cost = 0.0; string message; - EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks, - &fixed_cost, - &message)); - EXPECT_EQ(program.NumParameterBlocks(), 2); - EXPECT_EQ(program.NumResidualBlocks(), 2); + scoped_ptr<Program> reduced_program( + CHECK_NOTNULL(problem + .program() + .CreateReducedProgram(&removed_parameter_blocks, + &fixed_cost, + &message))); + EXPECT_EQ(reduced_program->NumParameterBlocks(), 2); + EXPECT_EQ(reduced_program->NumResidualBlocks(), 2); } TEST(Program, RemoveFixedBlocksFixedCost) { @@ -211,27 +222,29 @@ problem.AddResidualBlock(new BinaryCostFunction(), NULL, &x, &y); problem.SetParameterBlockConstant(&x); - Program program(problem.program()); - - double expected_fixed_cost; - ResidualBlock *expected_removed_block = program.residual_blocks()[0]; + ResidualBlock *expected_removed_block = problem.program().residual_blocks()[0]; scoped_array<double> scratch( new double[expected_removed_block->NumScratchDoublesForEvaluate()]); + double expected_fixed_cost; expected_removed_block->Evaluate(true, &expected_fixed_cost, NULL, NULL, scratch.get()); + vector<double*> removed_parameter_blocks; double fixed_cost = 0.0; string message; - EXPECT_TRUE(program.RemoveFixedBlocks(&removed_parameter_blocks, - &fixed_cost, - &message)); + scoped_ptr<Program> reduced_program( + CHECK_NOTNULL(problem + .program() + .CreateReducedProgram(&removed_parameter_blocks, + &fixed_cost, + &message))); - EXPECT_EQ(program.NumParameterBlocks(), 2); - EXPECT_EQ(program.NumResidualBlocks(), 2); + EXPECT_EQ(reduced_program->NumParameterBlocks(), 2); + EXPECT_EQ(reduced_program->NumResidualBlocks(), 2); EXPECT_DOUBLE_EQ(fixed_cost, expected_fixed_cost); }
diff --git a/internal/ceres/solver_impl.cc b/internal/ceres/solver_impl.cc index 11398b1..dfdfecb 100644 --- a/internal/ceres/solver_impl.cc +++ b/internal/ceres/solver_impl.cc
@@ -664,42 +664,44 @@ string* error) { CHECK_NOTNULL(options->linear_solver_ordering.get()); Program* original_program = problem_impl->mutable_program(); - scoped_ptr<Program> transformed_program(new Program(*original_program)); + + vector<double*> removed_parameter_blocks; + scoped_ptr<Program> reduced_program( + original_program->CreateReducedProgram(&removed_parameter_blocks, + fixed_cost, + error)); + if (reduced_program.get() == NULL) { + return NULL; + } + + VLOG(2) << "Reduced problem: " + << reduced_program->NumParameterBlocks() + << " parameter blocks, " + << reduced_program->NumParameters() + << " parameters, " + << reduced_program->NumResidualBlocks() + << " residual blocks, " + << reduced_program->NumResiduals() + << " residuals."; + + if (reduced_program->NumParameterBlocks() == 0) { + LOG(WARNING) << "No varying parameter blocks to optimize; " + << "bailing early."; + return reduced_program.release(); + } ParameterBlockOrdering* linear_solver_ordering = options->linear_solver_ordering.get(); const int min_group_id = linear_solver_ordering->group_to_elements().begin()->first; - vector<double*> removed_parameter_blocks; - if (!transformed_program->RemoveFixedBlocks(&removed_parameter_blocks, - fixed_cost, - error)) { - return NULL; - } - linear_solver_ordering->Remove(removed_parameter_blocks); + ParameterBlockOrdering* inner_iteration_ordering = options->inner_iteration_ordering.get(); if (inner_iteration_ordering != NULL) { inner_iteration_ordering->Remove(removed_parameter_blocks); } - VLOG(2) << "Reduced problem: " - << transformed_program->NumParameterBlocks() - << " parameter blocks, " - << transformed_program->NumParameters() - << " parameters, " - << transformed_program->NumResidualBlocks() - << " residual blocks, " - << transformed_program->NumResiduals() - << " residuals."; - - if (transformed_program->NumParameterBlocks() == 0) { - LOG(WARNING) << "No varying parameter blocks to optimize; " - << "bailing early."; - return transformed_program.release(); - } - if (IsSchurType(options->linear_solver_type) && linear_solver_ordering->GroupSize(min_group_id) == 0) { // If the user requested the use of a Schur type solver, and @@ -730,11 +732,11 @@ options->sparse_linear_algebra_library_type, problem_impl->parameter_map(), linear_solver_ordering, - transformed_program.get(), + reduced_program.get(), error)) { return NULL; } - return transformed_program.release(); + return reduced_program.release(); } if (options->linear_solver_type == SPARSE_NORMAL_CHOLESKY && @@ -742,16 +744,16 @@ if (!ReorderProgramForSparseNormalCholesky( options->sparse_linear_algebra_library_type, linear_solver_ordering, - transformed_program.get(), + reduced_program.get(), error)) { return NULL; } - return transformed_program.release(); + return reduced_program.release(); } - transformed_program->SetParameterOffsetsAndIndex(); - return transformed_program.release(); + reduced_program->SetParameterOffsetsAndIndex(); + return reduced_program.release(); } LinearSolver* SolverImpl::CreateLinearSolver(Solver::Options* options,