GradientProblem & related classes use std::unique_ptr
Previously these classes in analogy with ceres::Problem's interface
had interfaces to allow bare pointers as well as unique_ptrs. This
CL changes the API to always use unique_ptr, this is less error prone
and makes the default ownership semantics clearer.
Change-Id: I7577a90761f341c7e009c248c820f0fec2e6f32d
diff --git a/docs/source/gradient_solver.rst b/docs/source/gradient_solver.rst
index 4e3fc71..abde99c 100644
--- a/docs/source/gradient_solver.rst
+++ b/docs/source/gradient_solver.rst
@@ -54,9 +54,9 @@
class GradientProblem {
public:
- explicit GradientProblem(FirstOrderFunction* function);
- GradientProblem(FirstOrderFunction* function,
- Manifold* manifold);
+ explicit GradientProblem(std::unique_ptr<FirstOrderFunction> function);
+ GradientProblem(std::unique_ptr<FirstOrderFunction function,
+ std::unique_ptr<Manifold> manifold);
int NumParameters() const;
int NumTangentParameters() const;
bool Evaluate(const double* parameters, double* cost, double* gradient) const;
diff --git a/docs/source/gradient_tutorial.rst b/docs/source/gradient_tutorial.rst
index 2af44e1..6f7508b 100644
--- a/docs/source/gradient_tutorial.rst
+++ b/docs/source/gradient_tutorial.rst
@@ -45,9 +45,10 @@
return true;
}
- static ceres::FirstOrderFunction* Create() {
+ static std::unique_ptr<ceres::FirstOrderFunction> Create() {
constexpr int kNumParameters = 2;
- return new ceres::AutoDiffFirstOrderFunction<Rosenbrock, kNumParameters>();
+ return std::make_unique<
+ ceres::AutoDiffFirstOrderFunction<Rosenbrock, kNumParameters>>();
}
};
@@ -156,11 +157,12 @@
return true;
}
- static ceres::FirstOrderFunction* Create() {
+ static std::unique_ptr<ceres::FirstOrderFunction> Create() {
constexpr int kNumParameters = 2;
- return new ceres::NumericDiffFirstOrderFunction<Rosenbrock,
- ceres::CENTRAL,
- kNumParameters>();
+ return std::make_unique<
+ ceres::NumericDiffFirstOrderFunction<Rosenbrock,
+ ceres::CENTRAL,
+ kNumParameters>>();
}
};
diff --git a/examples/rosenbrock.cc b/examples/rosenbrock.cc
index df609a0..d739325 100644
--- a/examples/rosenbrock.cc
+++ b/examples/rosenbrock.cc
@@ -45,10 +45,10 @@
return true;
}
- static ceres::FirstOrderFunction* Create() {
+ static std::unique_ptr<ceres::FirstOrderFunction> Create() {
constexpr int kNumParameters = 2;
- return new ceres::AutoDiffFirstOrderFunction<Rosenbrock, kNumParameters>(
- new Rosenbrock);
+ return std::make_unique<
+ ceres::AutoDiffFirstOrderFunction<Rosenbrock, kNumParameters>>();
}
};
diff --git a/examples/rosenbrock_analytic_diff.cc b/examples/rosenbrock_analytic_diff.cc
index fb1856e..50e1de5 100644
--- a/examples/rosenbrock_analytic_diff.cc
+++ b/examples/rosenbrock_analytic_diff.cc
@@ -68,7 +68,7 @@
options.minimizer_progress_to_stdout = true;
ceres::GradientProblemSolver::Summary summary;
- ceres::GradientProblem problem(new Rosenbrock());
+ ceres::GradientProblem problem(std::make_unique<Rosenbrock>());
ceres::Solve(options, problem, parameters, &summary);
std::cout << summary.FullReport() << "\n";
diff --git a/examples/rosenbrock_numeric_diff.cc b/examples/rosenbrock_numeric_diff.cc
index 4115669..23be7f4 100644
--- a/examples/rosenbrock_numeric_diff.cc
+++ b/examples/rosenbrock_numeric_diff.cc
@@ -47,12 +47,12 @@
return true;
}
- static ceres::FirstOrderFunction* Create() {
+ static std::unique_ptr<ceres::FirstOrderFunction> Create() {
constexpr int kNumParameters = 2;
- return new ceres::NumericDiffFirstOrderFunction<Rosenbrock,
- ceres::CENTRAL,
- kNumParameters>(
- new Rosenbrock);
+ return std::make_unique<
+ ceres::NumericDiffFirstOrderFunction<Rosenbrock,
+ ceres::CENTRAL,
+ kNumParameters>>();
}
};
diff --git a/include/ceres/autodiff_first_order_function.h b/include/ceres/autodiff_first_order_function.h
index 2dca8f3..0e7def3 100644
--- a/include/ceres/autodiff_first_order_function.h
+++ b/include/ceres/autodiff_first_order_function.h
@@ -91,7 +91,7 @@
//
// FirstOrderFunction* function =
// new AutoDiffFirstOrderFunction<QuadraticCostFunctor, 4>(
-// new QuadraticCostFunctor(1.0)));
+// std::make_unique<QuadraticCostFunctor>(1.0)));
//
// In the instantiation above, the template parameters following
// "QuadraticCostFunctor", "4", describe the functor as computing a
@@ -105,10 +105,13 @@
template <typename FirstOrderFunctor, int kNumParameters>
class AutoDiffFirstOrderFunction final : public FirstOrderFunction {
public:
- // Takes ownership of functor.
- explicit AutoDiffFirstOrderFunction(FirstOrderFunctor* functor)
- : AutoDiffFirstOrderFunction{
- std::unique_ptr<FirstOrderFunctor>{functor}} {}
+ AutoDiffFirstOrderFunction(const AutoDiffFirstOrderFunction&) = delete;
+ AutoDiffFirstOrderFunction& operator=(const AutoDiffFirstOrderFunction&) =
+ delete;
+ AutoDiffFirstOrderFunction(AutoDiffFirstOrderFunction&& other) noexcept =
+ default;
+ AutoDiffFirstOrderFunction& operator=(
+ AutoDiffFirstOrderFunction&& other) noexcept = default;
explicit AutoDiffFirstOrderFunction(
std::unique_ptr<FirstOrderFunctor> functor)
diff --git a/include/ceres/gradient_problem.h b/include/ceres/gradient_problem.h
index 96d6493..3d8567c 100644
--- a/include/ceres/gradient_problem.h
+++ b/include/ceres/gradient_problem.h
@@ -88,14 +88,12 @@
// virtual int NumParameters() const { return 2; };
// };
//
-// ceres::GradientProblem problem(new Rosenbrock());
+// ceres::GradientProblem problem(std::make_unique<Rosenbrock>());
class CERES_EXPORT GradientProblem {
public:
- // Takes ownership of the function.
- explicit GradientProblem(FirstOrderFunction* function);
-
- // Takes ownership of the function and the manifold.
- GradientProblem(FirstOrderFunction* function, Manifold* manifold);
+ explicit GradientProblem(std::unique_ptr<FirstOrderFunction> function);
+ GradientProblem(std::unique_ptr<FirstOrderFunction> function,
+ std::unique_ptr<Manifold> manifold);
int NumParameters() const;
diff --git a/include/ceres/numeric_diff_first_order_function.h b/include/ceres/numeric_diff_first_order_function.h
index a9a7798..fbcb9a3 100644
--- a/include/ceres/numeric_diff_first_order_function.h
+++ b/include/ceres/numeric_diff_first_order_function.h
@@ -90,12 +90,13 @@
// first order function with central differences used for computing the
// derivative can be constructed as follows.
//
-// FirstOrderFunction* function
-// = new NumericDiffFirstOrderFunction<MyScalarCostFunctor, CENTRAL, 4>(
-// new QuadraticCostFunctor(1.0)); ^ ^ ^
-// | | |
-// Finite Differencing Scheme -+ | |
-// Dimension of xy ------------------------+
+// std::unique_ptr<FirstOrderFunction> function
+// = std::make_unique<
+// NumericDiffFirstOrderFunction<MyScalarCostFunctor, CENTRAL, 4>>(
+// std::make_unique<QuadraticCostFunctor>(1.0)); ^ ^
+// | |
+// Finite Differencing Scheme -----+ |
+// Dimension of xy ----------------------+
//
//
// In the instantiation above, the template parameters following
@@ -106,9 +107,10 @@
// If the size of the parameter vector is not known at compile time, then an
// alternate construction syntax can be used:
//
-// FirstOrderFunction* function
-// = new NumericDiffFirstOrderFunction<MyScalarCostFunctor, CENTRAL>(
-// new QuadraticCostFunctor(1.0), 4);
+// std::unique_ptr<FirstOrderFunction> function
+// = std::make_unique<NumericDiffFirstOrderFunction<MyScalarCostFunctor,
+// CENTRAL>>(
+// std::make_unique<QuadraticCostFunctor>(1.0), 4);
//
// Note that instead of passing 4 as a template argument, it is now passed as
// the second argument to the constructor.
@@ -117,15 +119,6 @@
int kNumParameters = DYNAMIC>
class NumericDiffFirstOrderFunction final : public FirstOrderFunction {
public:
- template <class... Args,
- bool kIsDynamic = kNumParameters == DYNAMIC,
- std::enable_if_t<!kIsDynamic &&
- std::is_constructible_v<FirstOrderFunctor,
- Args&&...>>* = nullptr>
- explicit NumericDiffFirstOrderFunction(Args&&... args)
- : NumericDiffFirstOrderFunction{std::make_unique<FirstOrderFunction>(
- std::forward<Args>(args)...)} {}
-
NumericDiffFirstOrderFunction(const NumericDiffFirstOrderFunction&) = delete;
NumericDiffFirstOrderFunction& operator=(
const NumericDiffFirstOrderFunction&) = delete;
@@ -136,35 +129,21 @@
// Constructor for the case where the parameter size is known at compile time.
explicit NumericDiffFirstOrderFunction(
- FirstOrderFunctor* functor,
- Ownership ownership = TAKE_OWNERSHIP,
- const NumericDiffOptions& options = NumericDiffOptions())
- : NumericDiffFirstOrderFunction{
- std::unique_ptr<FirstOrderFunctor>{functor},
- kNumParameters,
- ownership,
- options,
- FIXED_INIT} {}
-
- // Constructor for the case where the parameter size is known at compile time.
- explicit NumericDiffFirstOrderFunction(
std::unique_ptr<FirstOrderFunctor> functor,
const NumericDiffOptions& options = NumericDiffOptions())
+ : NumericDiffFirstOrderFunction{std::move(functor),
+ kNumParameters,
+ TAKE_OWNERSHIP,
+ options,
+ FIXED_INIT} {}
+ template <class... Args,
+ bool kIsDynamic = kNumParameters == DYNAMIC,
+ std::enable_if_t<!kIsDynamic &&
+ std::is_constructible_v<FirstOrderFunctor,
+ Args&&...>>* = nullptr>
+ explicit NumericDiffFirstOrderFunction(Args&&... args)
: NumericDiffFirstOrderFunction{
- std::move(functor), kNumParameters, TAKE_OWNERSHIP, FIXED_INIT} {}
-
- // Constructor for the case where the parameter size is specified at run time.
- explicit NumericDiffFirstOrderFunction(
- FirstOrderFunctor* functor,
- int num_parameters,
- Ownership ownership = TAKE_OWNERSHIP,
- const NumericDiffOptions& options = NumericDiffOptions())
- : NumericDiffFirstOrderFunction{
- std::unique_ptr<FirstOrderFunctor>{functor},
- num_parameters,
- ownership,
- options,
- DYNAMIC_INIT} {}
+ std::make_unique<FirstOrderFunctor>(std::forward<Args>(args)...)} {}
// Constructor for the case where the parameter size is specified at run time.
explicit NumericDiffFirstOrderFunction(
diff --git a/internal/ceres/autodiff_first_order_function_test.cc b/internal/ceres/autodiff_first_order_function_test.cc
index e663f13..5f1eb0b 100644
--- a/internal/ceres/autodiff_first_order_function_test.cc
+++ b/internal/ceres/autodiff_first_order_function_test.cc
@@ -53,9 +53,9 @@
};
TEST(AutoDiffFirstOrderFunction, BilinearDifferentiationTest) {
- std::unique_ptr<FirstOrderFunction> function(
- new AutoDiffFirstOrderFunction<QuadraticCostFunctor, 4>(
- new QuadraticCostFunctor(1.0)));
+ std::unique_ptr<FirstOrderFunction> function =
+ std::make_unique<AutoDiffFirstOrderFunction<QuadraticCostFunctor, 4>>(
+ 1.0);
double parameters[4] = {1.0, 2.0, 3.0, 4.0};
double gradient[4];
diff --git a/internal/ceres/gradient_problem.cc b/internal/ceres/gradient_problem.cc
index 486c99a..71434ba 100644
--- a/internal/ceres/gradient_problem.cc
+++ b/internal/ceres/gradient_problem.cc
@@ -36,24 +36,21 @@
namespace ceres {
-GradientProblem::GradientProblem(FirstOrderFunction* function)
- : function_(function),
+GradientProblem::GradientProblem(std::unique_ptr<FirstOrderFunction> function)
+ : function_(std::move(function)),
manifold_(std::make_unique<EuclideanManifold<DYNAMIC>>(
function_->NumParameters())),
scratch_(new double[function_->NumParameters()]) {
- CHECK(function != nullptr);
+ CHECK(function_ != nullptr);
}
-GradientProblem::GradientProblem(FirstOrderFunction* function,
- Manifold* manifold)
- : function_(function), scratch_(new double[function_->NumParameters()]) {
- CHECK(function != nullptr);
- if (manifold != nullptr) {
- manifold_.reset(manifold);
- } else {
- manifold_ = std::make_unique<EuclideanManifold<DYNAMIC>>(
- function_->NumParameters());
- }
+GradientProblem::GradientProblem(std::unique_ptr<FirstOrderFunction> function,
+ std::unique_ptr<Manifold> manifold)
+ : function_(std::move(function)),
+ manifold_(std::move(manifold)),
+ scratch_(new double[function_->NumParameters()]) {
+ CHECK(function_ != nullptr);
+ CHECK(manifold_ != nullptr);
CHECK_EQ(function_->NumParameters(), manifold_->AmbientSize());
}
diff --git a/internal/ceres/gradient_problem_solver_test.cc b/internal/ceres/gradient_problem_solver_test.cc
index f8eabf6..52884dc 100644
--- a/internal/ceres/gradient_problem_solver_test.cc
+++ b/internal/ceres/gradient_problem_solver_test.cc
@@ -61,7 +61,7 @@
ceres::GradientProblemSolver::Options options;
ceres::GradientProblemSolver::Summary summary;
- ceres::GradientProblem problem(new Rosenbrock());
+ ceres::GradientProblem problem(std::make_unique<Rosenbrock>());
ceres::Solve(options, problem, parameters, &summary);
EXPECT_EQ(CONVERGENCE, summary.termination_type);
@@ -99,7 +99,7 @@
double x = 50.0;
const double original_x = x;
- ceres::GradientProblem problem(new QuadraticFunction);
+ ceres::GradientProblem problem(std::make_unique<QuadraticFunction>());
ceres::GradientProblemSolver::Options options;
RememberingCallback callback(&x);
options.callbacks.push_back(&callback);
diff --git a/internal/ceres/gradient_problem_test.cc b/internal/ceres/gradient_problem_test.cc
index 52757a3..bebc3a7 100644
--- a/internal/ceres/gradient_problem_test.cc
+++ b/internal/ceres/gradient_problem_test.cc
@@ -64,13 +64,16 @@
TEST(GradientProblem, TakesOwnershipOfFirstOrderFunction) {
bool is_destructed = false;
- { ceres::GradientProblem problem(new QuadraticTestFunction(&is_destructed)); }
+ {
+ ceres::GradientProblem problem(
+ std::make_unique<QuadraticTestFunction>(&is_destructed));
+ }
EXPECT_TRUE(is_destructed);
}
TEST(GradientProblem, EvaluationWithManifoldAndNoGradient) {
- ceres::GradientProblem problem(new QuadraticTestFunction(),
- new EuclideanManifold<1>);
+ ceres::GradientProblem problem(std::make_unique<QuadraticTestFunction>(),
+ std::make_unique<EuclideanManifold<1>>());
double x = 7.0;
double cost = 0;
problem.Evaluate(&x, &cost, nullptr);
@@ -78,7 +81,7 @@
}
TEST(GradientProblem, EvaluationWithoutManifoldAndWithGradient) {
- ceres::GradientProblem problem(new QuadraticTestFunction());
+ ceres::GradientProblem problem(std::make_unique<QuadraticTestFunction>());
double x = 7.0;
double cost = 0;
double gradient = 0;
@@ -87,8 +90,8 @@
}
TEST(GradientProblem, EvaluationWithManifoldAndWithGradient) {
- ceres::GradientProblem problem(new QuadraticTestFunction(),
- new EuclideanManifold<1>);
+ ceres::GradientProblem problem(std::make_unique<QuadraticTestFunction>(),
+ std::make_unique<EuclideanManifold<1>>());
double x = 7.0;
double cost = 0;
double gradient = 0;
diff --git a/internal/ceres/line_search_minimizer_test.cc b/internal/ceres/line_search_minimizer_test.cc
index 576d90e..5225f7b 100644
--- a/internal/ceres/line_search_minimizer_test.cc
+++ b/internal/ceres/line_search_minimizer_test.cc
@@ -52,7 +52,8 @@
TEST(LineSearchMinimizerTest, FinalCostIsZero) {
double parameters[1] = {2.0};
- ceres::GradientProblem problem(new QuadraticFirstOrderFunction);
+ ceres::GradientProblem problem(
+ std::make_unique<QuadraticFirstOrderFunction>());
ceres::GradientProblemSolver::Options options;
ceres::GradientProblemSolver::Summary summary;
ceres::Solve(options, problem, parameters, &summary);
diff --git a/internal/ceres/numeric_diff_first_order_function_test.cc b/internal/ceres/numeric_diff_first_order_function_test.cc
index ff57e2d..c294627 100644
--- a/internal/ceres/numeric_diff_first_order_function_test.cc
+++ b/internal/ceres/numeric_diff_first_order_function_test.cc
@@ -52,8 +52,7 @@
TEST(NumericDiffFirstOrderFunction, BilinearDifferentiationTestStatic) {
auto function = std::make_unique<
- NumericDiffFirstOrderFunction<QuadraticCostFunctor, CENTRAL, 4>>(
- new QuadraticCostFunctor(1.0));
+ NumericDiffFirstOrderFunction<QuadraticCostFunctor, CENTRAL, 4>>(1.0);
double parameters[4] = {1.0, 2.0, 3.0, 4.0};
double gradient[4];
@@ -77,7 +76,7 @@
TEST(NumericDiffFirstOrderFunction, BilinearDifferentiationTestDynamic) {
auto function = std::make_unique<
NumericDiffFirstOrderFunction<QuadraticCostFunctor, CENTRAL>>(
- new QuadraticCostFunctor(1.0), 4);
+ std::make_unique<QuadraticCostFunctor>(1.0), 4);
double parameters[4] = {1.0, 2.0, 3.0, 4.0};
double gradient[4];
