Change ownership of pointers in Minimizer::Options.
This is a intermediate change to clean things up
in preparation for a broader refactoring of the SolverImpl.
Essentially we are replacing raw pointers in Minimizer::Options
with shared_ptr objects. For now this only makes things a bit
more complicated looking inside solver_impl.cc, but going
forward this will lead to considerable simplifications in
tracking ownership of various pointers.
Change-Id: I21db8fc6763c29b0d15e834d7c968a0f514042a0
diff --git a/internal/ceres/coordinate_descent_minimizer.cc b/internal/ceres/coordinate_descent_minimizer.cc
index 3b0553e..1d55458 100644
--- a/internal/ceres/coordinate_descent_minimizer.cc
+++ b/internal/ceres/coordinate_descent_minimizer.cc
@@ -210,23 +210,16 @@
summary->final_cost = 0.0;
string error;
- scoped_ptr<Evaluator> evaluator(
- Evaluator::Create(evaluator_options_, program, &error));
- CHECK_NOTNULL(evaluator.get());
-
- scoped_ptr<SparseMatrix> jacobian(evaluator->CreateJacobian());
- CHECK_NOTNULL(jacobian.get());
+ Minimizer::Options minimizer_options;
+ minimizer_options.evaluator.reset(
+ CHECK_NOTNULL(Evaluator::Create(evaluator_options_, program, &error)));
+ minimizer_options.jacobian.reset(
+ CHECK_NOTNULL(minimizer_options.evaluator->CreateJacobian()));
TrustRegionStrategy::Options trs_options;
trs_options.linear_solver = linear_solver;
-
- scoped_ptr<TrustRegionStrategy>trust_region_strategy(
+ minimizer_options.trust_region_strategy.reset(
CHECK_NOTNULL(TrustRegionStrategy::Create(trs_options)));
-
- Minimizer::Options minimizer_options;
- minimizer_options.evaluator = evaluator.get();
- minimizer_options.jacobian = jacobian.get();
- minimizer_options.trust_region_strategy = trust_region_strategy.get();
minimizer_options.is_silent = true;
TrustRegionMinimizer minimizer;
diff --git a/internal/ceres/coordinate_descent_minimizer.h b/internal/ceres/coordinate_descent_minimizer.h
index e324b38..c1f8ffc 100644
--- a/internal/ceres/coordinate_descent_minimizer.h
+++ b/internal/ceres/coordinate_descent_minimizer.h
@@ -43,6 +43,7 @@
namespace internal {
class Program;
+class LinearSolver;
// Given a Program, and a ParameterBlockOrdering which partitions
// (non-exhaustively) the Hessian matrix into independent sets,
diff --git a/internal/ceres/line_search_minimizer.cc b/internal/ceres/line_search_minimizer.cc
index ae77a73..f494eda 100644
--- a/internal/ceres/line_search_minimizer.cc
+++ b/internal/ceres/line_search_minimizer.cc
@@ -103,7 +103,7 @@
double start_time = WallTimeInSeconds();
double iteration_start_time = start_time;
- Evaluator* evaluator = CHECK_NOTNULL(options.evaluator);
+ Evaluator* evaluator = CHECK_NOTNULL(options.evaluator.get());
const int num_parameters = evaluator->NumParameters();
const int num_effective_parameters = evaluator->NumEffectiveParameters();
diff --git a/internal/ceres/minimizer.cc b/internal/ceres/minimizer.cc
index 6c3b68d..558921b 100644
--- a/internal/ceres/minimizer.cc
+++ b/internal/ceres/minimizer.cc
@@ -28,13 +28,29 @@
//
// Author: sameeragarwal@google.com (Sameer Agarwal)
+#include "ceres/line_search_minimizer.h"
#include "ceres/minimizer.h"
+#include "ceres/trust_region_minimizer.h"
#include "ceres/types.h"
#include "glog/logging.h"
namespace ceres {
namespace internal {
+Minimizer* Minimizer::Create(MinimizerType minimizer_type) {
+ if (minimizer_type == TRUST_REGION) {
+ return new TrustRegionMinimizer;
+ }
+
+ if (minimizer_type == LINE_SEARCH) {
+ return new LineSearchMinimizer;
+ }
+
+ LOG(FATAL) << "Unknown minimizer_type: " << minimizer_type;
+ return NULL;
+}
+
+
Minimizer::~Minimizer() {}
bool Minimizer::RunCallbacks(const Minimizer::Options& options,
diff --git a/internal/ceres/minimizer.h b/internal/ceres/minimizer.h
index f1da3f7..dabf07e 100644
--- a/internal/ceres/minimizer.h
+++ b/internal/ceres/minimizer.h
@@ -41,9 +41,10 @@
namespace internal {
class Evaluator;
-class LinearSolver;
class SparseMatrix;
class TrustRegionStrategy;
+class CoordinateDescentMinimizer;
+class LinearSolver;
// Interface for non-linear least squares solvers.
class Minimizer {
@@ -107,14 +108,10 @@
options.line_search_sufficient_curvature_decrease;
max_line_search_step_expansion =
options.max_line_search_step_expansion;
- is_silent = (options.logging_type == SILENT);
- evaluator = NULL;
- trust_region_strategy = NULL;
- jacobian = NULL;
- callbacks = options.callbacks;
- inner_iteration_minimizer = NULL;
inner_iteration_tolerance = options.inner_iteration_tolerance;
+ is_silent = (options.logging_type == SILENT);
is_constrained = false;
+ callbacks = options.callbacks;
}
int max_num_iterations;
@@ -154,10 +151,14 @@
int max_num_line_search_direction_restarts;
double line_search_sufficient_curvature_decrease;
double max_line_search_step_expansion;
+ double inner_iteration_tolerance;
// If true, then all logging is disabled.
bool is_silent;
+ // Use a bounds constrained optimization algorithm.
+ bool is_constrained;
+
// List of callbacks that are executed by the Minimizer at the end
// of each iteration.
//
@@ -165,27 +166,23 @@
vector<IterationCallback*> callbacks;
// Object responsible for evaluating the cost, residuals and
- // Jacobian matrix. The Options struct does not own this pointer.
- Evaluator* evaluator;
+ // Jacobian matrix.
+ shared_ptr<Evaluator> evaluator;
// Object responsible for actually computing the trust region
- // step, and sizing the trust region radius. The Options struct
- // does not own this pointer.
- TrustRegionStrategy* trust_region_strategy;
+ // step, and sizing the trust region radius.
+ shared_ptr<TrustRegionStrategy> trust_region_strategy;
// Object holding the Jacobian matrix. It is assumed that the
// sparsity structure of the matrix has already been initialized
// and will remain constant for the life time of the
- // optimization. The Options struct does not own this pointer.
- SparseMatrix* jacobian;
+ // optimization.
+ shared_ptr<SparseMatrix> jacobian;
- Minimizer* inner_iteration_minimizer;
- double inner_iteration_tolerance;
-
- // Use a bounds constrained optimization algorithm.
- bool is_constrained;
+ shared_ptr<CoordinateDescentMinimizer> inner_iteration_minimizer;
};
+ static Minimizer* Create(MinimizerType minimizer_type);
static bool RunCallbacks(const Options& options,
const IterationSummary& iteration_summary,
Solver::Summary* summary);
diff --git a/internal/ceres/solver_impl.cc b/internal/ceres/solver_impl.cc
index a1cf4ca..41dbcde 100644
--- a/internal/ceres/solver_impl.cc
+++ b/internal/ceres/solver_impl.cc
@@ -67,8 +67,8 @@
void SolverImpl::TrustRegionMinimize(
const Solver::Options& options,
Program* program,
- CoordinateDescentMinimizer* inner_iteration_minimizer,
- Evaluator* evaluator,
+ shared_ptr<CoordinateDescentMinimizer> inner_iteration_minimizer,
+ shared_ptr<Evaluator> evaluator,
LinearSolver* linear_solver,
Solver::Summary* summary) {
Minimizer::Options minimizer_options(options);
@@ -98,9 +98,7 @@
}
minimizer_options.evaluator = evaluator;
- scoped_ptr<SparseMatrix> jacobian(evaluator->CreateJacobian());
-
- minimizer_options.jacobian = jacobian.get();
+ minimizer_options.jacobian.reset(evaluator->CreateJacobian());
minimizer_options.inner_iteration_minimizer = inner_iteration_minimizer;
TrustRegionStrategy::Options trust_region_strategy_options;
@@ -113,9 +111,7 @@
trust_region_strategy_options.trust_region_strategy_type =
options.trust_region_strategy_type;
trust_region_strategy_options.dogleg_type = options.dogleg_type;
- scoped_ptr<TrustRegionStrategy> strategy(
- TrustRegionStrategy::Create(trust_region_strategy_options));
- minimizer_options.trust_region_strategy = strategy.get();
+ minimizer_options.trust_region_strategy.reset(TrustRegionStrategy::Create(trust_region_strategy_options));
TrustRegionMinimizer minimizer;
double minimizer_start_time = WallTimeInSeconds();
@@ -137,7 +133,7 @@
void SolverImpl::LineSearchMinimize(
const Solver::Options& options,
Program* program,
- Evaluator* evaluator,
+ shared_ptr<Evaluator> evaluator,
Solver::Summary* summary) {
Minimizer::Options minimizer_options(options);
@@ -364,18 +360,18 @@
summary->trust_region_strategy_type = options.trust_region_strategy_type;
summary->dogleg_type = options.dogleg_type;
- scoped_ptr<Evaluator> evaluator(CreateEvaluator(options,
+ shared_ptr<Evaluator> evaluator(CreateEvaluator(options,
problem_impl->parameter_map(),
reduced_program.get(),
&summary->message));
event_logger.AddEvent("CreateEvaluator");
- if (evaluator == NULL) {
+ if (evaluator.get() == NULL) {
return;
}
- scoped_ptr<CoordinateDescentMinimizer> inner_iteration_minimizer;
+ shared_ptr<CoordinateDescentMinimizer> inner_iteration_minimizer;
if (options.use_inner_iterations) {
if (reduced_program->parameter_blocks().size() < 2) {
LOG(WARNING) << "Reduced problem only contains one parameter block."
@@ -386,7 +382,7 @@
*reduced_program,
problem_impl->parameter_map(),
summary));
- if (inner_iteration_minimizer == NULL) {
+ if (inner_iteration_minimizer.get() == NULL) {
LOG(ERROR) << summary->message;
return;
}
@@ -401,8 +397,8 @@
// Run the optimization.
TrustRegionMinimize(options,
reduced_program.get(),
- inner_iteration_minimizer.get(),
- evaluator.get(),
+ inner_iteration_minimizer,
+ evaluator,
linear_solver.get(),
summary);
event_logger.AddEvent("Minimize");
@@ -561,11 +557,11 @@
return;
}
- scoped_ptr<Evaluator> evaluator(CreateEvaluator(options,
+ shared_ptr<Evaluator> evaluator(CreateEvaluator(options,
problem_impl->parameter_map(),
reduced_program.get(),
&summary->message));
- if (evaluator == NULL) {
+ if (evaluator.get() == NULL) {
return;
}
@@ -574,7 +570,7 @@
minimizer_start_time - solver_start_time;
// Run the optimization.
- LineSearchMinimize(options, reduced_program.get(), evaluator.get(), summary);
+ LineSearchMinimize(options, reduced_program.get(), evaluator, summary);
const double post_process_start_time = WallTimeInSeconds();
diff --git a/internal/ceres/solver_impl.h b/internal/ceres/solver_impl.h
index c42c32a..5cefdb1 100644
--- a/internal/ceres/solver_impl.h
+++ b/internal/ceres/solver_impl.h
@@ -64,8 +64,8 @@
static void TrustRegionMinimize(
const Solver::Options &options,
Program* program,
- CoordinateDescentMinimizer* inner_iteration_minimizer,
- Evaluator* evaluator,
+ shared_ptr<CoordinateDescentMinimizer> inner_iteration_minimizer,
+ shared_ptr<Evaluator> evaluator,
LinearSolver* linear_solver,
Solver::Summary* summary);
@@ -76,7 +76,7 @@
// Run the LineSearchMinimizer for the given evaluator and configuration.
static void LineSearchMinimize(const Solver::Options &options,
Program* program,
- Evaluator* evaluator,
+ shared_ptr<Evaluator> evaluator,
Solver::Summary* summary);
// Create the transformed Program, which has all the fixed blocks
diff --git a/internal/ceres/trust_region_minimizer.cc b/internal/ceres/trust_region_minimizer.cc
index 4be5619..52e3b23 100644
--- a/internal/ceres/trust_region_minimizer.cc
+++ b/internal/ceres/trust_region_minimizer.cc
@@ -1,5 +1,5 @@
// Ceres Solver - A fast non-linear least squares minimizer
-// Copyright 2012 Google Inc. All rights reserved.
+// Copyright 2014 Google Inc. All rights reserved.
// http://code.google.com/p/ceres-solver/
//
// Redistribution and use in source and binary forms, with or without
@@ -40,6 +40,7 @@
#include "Eigen/Core"
#include "ceres/array_utils.h"
+#include "ceres/coordinate_descent_minimizer.h"
#include "ceres/evaluator.h"
#include "ceres/file.h"
#include "ceres/internal/eigen.h"
@@ -128,9 +129,9 @@
double iteration_start_time = start_time;
Init(options);
- Evaluator* evaluator = CHECK_NOTNULL(options_.evaluator);
- SparseMatrix* jacobian = CHECK_NOTNULL(options_.jacobian);
- TrustRegionStrategy* strategy = CHECK_NOTNULL(options_.trust_region_strategy);
+ Evaluator* evaluator = CHECK_NOTNULL(options_.evaluator.get());
+ SparseMatrix* jacobian = CHECK_NOTNULL(options_.jacobian.get());
+ TrustRegionStrategy* strategy = CHECK_NOTNULL(options_.trust_region_strategy.get());
const bool is_not_silent = !options.is_silent;
@@ -253,7 +254,8 @@
double candidate_cost = cost;
double accumulated_candidate_model_cost_change = 0.0;
int num_consecutive_invalid_steps = 0;
- bool inner_iterations_are_enabled = options.inner_iteration_minimizer != NULL;
+ bool inner_iterations_are_enabled =
+ options.inner_iteration_minimizer.get() != NULL;
while (true) {
bool inner_iterations_were_useful = false;
if (!RunCallbacks(options, iteration_summary, summary)) {
diff --git a/internal/ceres/trust_region_minimizer_test.cc b/internal/ceres/trust_region_minimizer_test.cc
index ef49206..94be541 100644
--- a/internal/ceres/trust_region_minimizer_test.cc
+++ b/internal/ceres/trust_region_minimizer_test.cc
@@ -223,15 +223,12 @@
parameters[2] = (col3 ? parameters[2] : 0.0);
parameters[3] = (col4 ? parameters[3] : 0.0);
- PowellEvaluator2<col1, col2, col3, col4> powell_evaluator;
- scoped_ptr<SparseMatrix> jacobian(powell_evaluator.CreateJacobian());
-
Minimizer::Options minimizer_options(solver_options);
minimizer_options.gradient_tolerance = 1e-26;
minimizer_options.function_tolerance = 1e-26;
minimizer_options.parameter_tolerance = 1e-26;
- minimizer_options.evaluator = &powell_evaluator;
- minimizer_options.jacobian = jacobian.get();
+ minimizer_options.evaluator.reset(new PowellEvaluator2<col1, col2, col3, col4>);
+ minimizer_options.jacobian.reset(minimizer_options.evaluator->CreateJacobian());
TrustRegionStrategy::Options trust_region_strategy_options;
trust_region_strategy_options.trust_region_strategy_type = strategy_type;
@@ -240,9 +237,8 @@
trust_region_strategy_options.max_radius = 1e20;
trust_region_strategy_options.min_lm_diagonal = 1e-6;
trust_region_strategy_options.max_lm_diagonal = 1e32;
- scoped_ptr<TrustRegionStrategy> strategy(
+ minimizer_options.trust_region_strategy.reset(
TrustRegionStrategy::Create(trust_region_strategy_options));
- minimizer_options.trust_region_strategy = strategy.get();
TrustRegionMinimizer minimizer;
Solver::Summary summary;
