Fix a logging bug in TrustRegionMinimizer.
Upon encountering an unsuccessful step (one where the cost goes up)
the the trust region minimizer failed to populate the gradient norm
in the IterationSummary. This would cause the gradient norm to be
logged as zero which is incorrect. Instead it should be the gradient
norm at the current point.
This CL fixes this issue.
Before:
iter cost cost_change |gradient| |step| tr_ratio tr_radius ls_iter iter_time total_time
0 1.115206e+07 0.00e+00 1.90e+07 0.00e+00 0.00e+00 1.00e+04 0 2.72e-01 1.33e+00
1 3.687552e+06 7.46e+06 1.84e+08 2.86e+03 6.91e-01 1.06e+04 1 1.32e+00 2.65e+00
2 3.670266e+10 -3.67e+10 0.00e+00 3.27e+03 -1.07e+04 5.30e+03 1 7.52e-01 3.40e+00
3 4.335397e+07 -3.97e+07 0.00e+00 2.74e+03 -1.16e+01 1.32e+03 1 7.28e-01 4.13e+00
4 1.345488e+06 2.34e+06 4.12e+07 1.55e+03 6.87e-01 1.40e+03 1 9.31e-01 5.06e+00
5 5.376653e+05 8.08e+05 9.99e+06 6.64e+02 7.46e-01 1.59e+03 1 9.64e-01 6.03e+00
After:
iter cost cost_change |gradient| |step| tr_ratio tr_radius ls_iter iter_time total_time
0 1.115206e+07 0.00e+00 1.90e+07 0.00e+00 0.00e+00 1.00e+04 0 2.37e-01 1.13e+00
1 3.687552e+06 7.46e+06 1.84e+08 2.86e+03 6.91e-01 1.06e+04 1 1.08e+00 2.21e+00
2 3.670266e+10 -3.67e+10 1.84e+08 3.27e+03 -1.07e+04 5.30e+03 1 7.50e-01 2.96e+00
3 4.335397e+07 -3.97e+07 1.84e+08 2.74e+03 -1.16e+01 1.32e+03 1 7.13e-01 3.67e+00
4 1.345488e+06 2.34e+06 4.12e+07 1.55e+03 6.87e-01 1.40e+03 1 9.01e-01 4.57e+00
5 5.376653e+05 8.08e+05 9.99e+06 6.64e+02 7.46e-01 1.59e+03 1 9.36e-01 5.51e+00
Change-Id: Iae538fe089be07c7bb219337a6f1392f7213acfe
diff --git a/internal/ceres/trust_region_minimizer.cc b/internal/ceres/trust_region_minimizer.cc
index 7065977..196f9e6 100644
--- a/internal/ceres/trust_region_minimizer.cc
+++ b/internal/ceres/trust_region_minimizer.cc
@@ -83,6 +83,10 @@
while (FinalizeIterationAndCheckIfMinimizerCanContinue()) {
iteration_start_time_in_secs_ = WallTimeInSeconds();
+
+ const double previous_gradient_norm = iteration_summary_.gradient_norm;
+ const double previous_gradient_max_norm = iteration_summary_.gradient_max_norm;
+
iteration_summary_ = IterationSummary();
iteration_summary_.iteration =
solver_summary->iterations.back().iteration + 1;
@@ -111,12 +115,22 @@
return;
}
+
+
if (IsStepSuccessful()) {
RETURN_IF_ERROR_AND_LOG(HandleSuccessfulStep());
- continue;
- }
+ } else {
+ // Declare the step unsuccessful and inform the trust region strategy.
+ iteration_summary_.step_is_successful = false;
+ iteration_summary_.cost = candidate_cost_ + solver_summary_->fixed_cost;
- HandleUnsuccessfulStep();
+ // When the step is unsuccessful, we do not compute the gradient
+ // (or update x), so we preserve its value from the last
+ // successful iteration.
+ iteration_summary_.gradient_norm = previous_gradient_norm;
+ iteration_summary_.gradient_max_norm = previous_gradient_max_norm;
+ strategy_->StepRejected(iteration_summary_.relative_decrease);
+ }
}
}
@@ -793,12 +807,5 @@
return true;
}
-// Declare the step unsuccessful and inform the trust region strategy.
-void TrustRegionMinimizer::HandleUnsuccessfulStep() {
- iteration_summary_.step_is_successful = false;
- strategy_->StepRejected(iteration_summary_.relative_decrease);
- iteration_summary_.cost = candidate_cost_ + solver_summary_->fixed_cost;
-}
-
} // namespace internal
} // namespace ceres
diff --git a/internal/ceres/trust_region_minimizer.h b/internal/ceres/trust_region_minimizer.h
index e18193c..27a1b78 100644
--- a/internal/ceres/trust_region_minimizer.h
+++ b/internal/ceres/trust_region_minimizer.h
@@ -81,7 +81,6 @@
bool MinTrustRegionRadiusReached();
bool IsStepSuccessful();
- void HandleUnsuccessfulStep();
bool HandleSuccessfulStep();
bool HandleInvalidStep();
