From 9beea728f6c1007483ce81e0f5add57860a05a1a Mon Sep 17 00:00:00 2001 From: Sameer Agarwal Date: Sat, 17 Dec 2022 16:57:02 -0800 Subject: [PATCH] Fix a bug in CoordinateDescentMinimizer CoordinateDescentMinimizer optimizes one parameter block at a time. To do this, it manipulates the parameter block object. It was doing so inconsistently, where the tangent space offset was being set to zero but the ambient state offset was not being set to zero. This did not cause problems because these offsets were not really being used inside the CoordinateDescentMinimizer. However the recent change which parallelizes Program::Plus uncovered this bug. The reason this bug was not caught was because, CoordinateDescentMinimizer does not have any tests. I will fix this shortly, but in the interim to unbreak inner iterations at head, this small change should go in. Change-Id: I55d2698e8509f9cb5751e7a5180427129d86e720 --- internal/ceres/coordinate_descent_minimizer.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/ceres/coordinate_descent_minimizer.cc b/internal/ceres/coordinate_descent_minimizer.cc index 8ecdeff47..de2282246 100644 --- a/internal/ceres/coordinate_descent_minimizer.cc +++ b/internal/ceres/coordinate_descent_minimizer.cc @@ -167,9 +167,11 @@ void CoordinateDescentMinimizer::Minimize(const Minimizer::Options& options, ParameterBlock* parameter_block = parameter_blocks_[j]; const int old_index = parameter_block->index(); const int old_delta_offset = parameter_block->delta_offset(); + const int old_state_offset = parameter_block->state_offset(); parameter_block->SetVarying(); parameter_block->set_index(0); parameter_block->set_delta_offset(0); + parameter_block->set_state_offset(0); Program inner_program; inner_program.mutable_parameter_blocks()->push_back(parameter_block); @@ -186,11 +188,12 @@ void CoordinateDescentMinimizer::Minimize(const Minimizer::Options& options, Solver::Summary inner_summary; Solve(&inner_program, linear_solvers[thread_id].get(), - parameters + parameter_block->state_offset(), + parameters + old_state_offset, &inner_summary); parameter_block->set_index(old_index); parameter_block->set_delta_offset(old_delta_offset); + parameter_block->set_state_offset(old_state_offset); parameter_block->SetState(parameters + parameter_block->state_offset()); parameter_block->SetConstant();