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

docs/source/gradient_solver.rst

@@ -54,9 +54,9 @@ Modeling
 
   class GradientProblem {
    public:
-    explicit GradientProblem(FirstOrderFunction* function);
+    explicit GradientProblem(std::unique_ptr<FirstOrderFunction> function);
-    GradientProblem(FirstOrderFunction* function,
+    GradientProblem(std::unique_ptr<FirstOrderFunction function,
-                    Manifold* manifold);
+                    std::unique_ptr<Manifold> manifold);
     int NumParameters() const;
     int NumTangentParameters() const;
     bool Evaluate(const double* parameters, double* cost, double* gradient) const;

docs/source/gradient_tutorial.rst

@@ -45,9 +45,10 @@ in Ceres.
       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 @@ follows [#f2]_.
       return true;
     }
 
-    static ceres::FirstOrderFunction* Create() {
+    static std::unique_ptr<ceres::FirstOrderFunction> Create() {
       constexpr int kNumParameters = 2;
-      return new ceres::NumericDiffFirstOrderFunction<Rosenbrock,
+      return std::make_unique<
+          ceres::NumericDiffFirstOrderFunction<Rosenbrock,
                                                ceres::CENTRAL,
-                                                      kNumParameters>();
+                                               kNumParameters>>();
     }
   };
 

examples/rosenbrock.cc

@@ -45,10 +45,10 @@ struct Rosenbrock {
     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<
-        new Rosenbrock);
+        ceres::AutoDiffFirstOrderFunction<Rosenbrock, kNumParameters>>();
   }
 };
 

examples/rosenbrock_analytic_diff.cc

@@ -68,7 +68,7 @@ int main(int argc, char** argv) {
   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";

examples/rosenbrock_numeric_diff.cc

@@ -47,12 +47,12 @@ struct Rosenbrock {
     return true;
   }
 
-  static ceres::FirstOrderFunction* Create() {
+  static std::unique_ptr<ceres::FirstOrderFunction> Create() {
     constexpr int kNumParameters = 2;
-    return new ceres::NumericDiffFirstOrderFunction<Rosenbrock,
+    return std::make_unique<
+        ceres::NumericDiffFirstOrderFunction<Rosenbrock,
                                              ceres::CENTRAL,
-                                                    kNumParameters>(
+                                             kNumParameters>>();
-        new Rosenbrock);
   }
 };
 

include/ceres/autodiff_first_order_function.h

@@ -91,7 +91,7 @@ namespace ceres {
 //
 //    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 @@ namespace ceres {
 template <typename FirstOrderFunctor, int kNumParameters>
 class AutoDiffFirstOrderFunction final : public FirstOrderFunction {
  public:
-  // Takes ownership of functor.
+  AutoDiffFirstOrderFunction(const AutoDiffFirstOrderFunction&) = delete;
-  explicit AutoDiffFirstOrderFunction(FirstOrderFunctor* functor)
+  AutoDiffFirstOrderFunction& operator=(const AutoDiffFirstOrderFunction&) =
-      : AutoDiffFirstOrderFunction{
+      delete;
-            std::unique_ptr<FirstOrderFunctor>{functor}} {}
+  AutoDiffFirstOrderFunction(AutoDiffFirstOrderFunction&& other) noexcept =
+      default;
+  AutoDiffFirstOrderFunction& operator=(
+      AutoDiffFirstOrderFunction&& other) noexcept = default;
 
   explicit AutoDiffFirstOrderFunction(
       std::unique_ptr<FirstOrderFunctor> functor)

include/ceres/gradient_problem.h

@@ -88,14 +88,12 @@ class FirstOrderFunction;
 //   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(std::unique_ptr<FirstOrderFunction> function);
-  explicit GradientProblem(FirstOrderFunction* function);
+  GradientProblem(std::unique_ptr<FirstOrderFunction> function,
-
+                  std::unique_ptr<Manifold> manifold);
-  // Takes ownership of the function and the manifold.
-  GradientProblem(FirstOrderFunction* function, Manifold* manifold);
 
   int NumParameters() const;
 

include/ceres/numeric_diff_first_order_function.h

@@ -90,12 +90,13 @@ namespace ceres {
 // first order function with central differences used for computing the
 // derivative can be constructed as follows.
 //
-//   FirstOrderFunction* function
+//   std::unique_ptr<FirstOrderFunction> function
-//       = new NumericDiffFirstOrderFunction<MyScalarCostFunctor, CENTRAL, 4>(
+//       = std::make_unique<
-//           new QuadraticCostFunctor(1.0));                   ^     ^     ^
+//           NumericDiffFirstOrderFunction<MyScalarCostFunctor, CENTRAL, 4>>(
-//                                                             |     |     |
+//               std::make_unique<QuadraticCostFunctor>(1.0));     ^     ^
-//                                 Finite Differencing Scheme -+     |     |
+//                                                                 |     |
-//                                 Dimension of xy ------------------------+
+//                                 Finite Differencing Scheme -----+     |
+//                                 Dimension of xy ----------------------+
 //
 //
 // In the instantiation above, the template parameters following
@@ -106,9 +107,10 @@ namespace ceres {
 // If the size of the parameter vector is not known at compile time, then an
 // alternate construction syntax can be used:
 //
-//   FirstOrderFunction* function
+//   std::unique_ptr<FirstOrderFunction> function
-//       = new NumericDiffFirstOrderFunction<MyScalarCostFunctor, CENTRAL>(
+//       = std::make_unique<NumericDiffFirstOrderFunction<MyScalarCostFunctor,
-//           new QuadraticCostFunctor(1.0), 4);
+//                                                        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 @@ template <typename FirstOrderFunctor,
           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;
@@ -134,37 +127,23 @@ class NumericDiffFirstOrderFunction final : public FirstOrderFunction {
   NumericDiffFirstOrderFunction& operator=(
       NumericDiffFirstOrderFunction&& other) noexcept = default;
 
-  // 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{
+      : NumericDiffFirstOrderFunction{std::move(functor),
-            std::move(functor), kNumParameters, TAKE_OWNERSHIP, FIXED_INIT} {}
+                                      kNumParameters,
-
+                                      TAKE_OWNERSHIP,
-  // 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} {}
+                                      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::make_unique<FirstOrderFunctor>(std::forward<Args>(args)...)} {}
 
   // Constructor for the case where the parameter size is specified at run time.
   explicit NumericDiffFirstOrderFunction(

internal/ceres/autodiff_first_order_function_test.cc

@@ -53,9 +53,9 @@ class QuadraticCostFunctor {
 };
 
 TEST(AutoDiffFirstOrderFunction, BilinearDifferentiationTest) {
-  std::unique_ptr<FirstOrderFunction> function(
+  std::unique_ptr<FirstOrderFunction> function =
-      new AutoDiffFirstOrderFunction<QuadraticCostFunctor, 4>(
+      std::make_unique<AutoDiffFirstOrderFunction<QuadraticCostFunctor, 4>>(
-          new QuadraticCostFunctor(1.0)));
+          1.0);
 
   double parameters[4] = {1.0, 2.0, 3.0, 4.0};
   double gradient[4];

internal/ceres/gradient_problem.cc

@@ -36,24 +36,21 @@
 
 namespace ceres {
 
-GradientProblem::GradientProblem(FirstOrderFunction* function)
+GradientProblem::GradientProblem(std::unique_ptr<FirstOrderFunction> function)
-    : function_(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,
+GradientProblem::GradientProblem(std::unique_ptr<FirstOrderFunction> function,
-                                 Manifold* manifold)
+                                 std::unique_ptr<Manifold> manifold)
-    : function_(function), scratch_(new double[function_->NumParameters()]) {
+    : function_(std::move(function)),
-  CHECK(function != nullptr);
+      manifold_(std::move(manifold)),
-  if (manifold != nullptr) {
+      scratch_(new double[function_->NumParameters()]) {
-    manifold_.reset(manifold);
+  CHECK(function_ != nullptr);
-  } else {
+  CHECK(manifold_ != nullptr);
-    manifold_ = std::make_unique<EuclideanManifold<DYNAMIC>>(
-        function_->NumParameters());
-  }
   CHECK_EQ(function_->NumParameters(), manifold_->AmbientSize());
 }
 

internal/ceres/gradient_problem_solver_test.cc

@@ -61,7 +61,7 @@ TEST(GradientProblemSolver, SolvesRosenbrockWithDefaultOptions) {
 
   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 @@ TEST(Solver, UpdateStateEveryIterationOption) {
   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);

internal/ceres/gradient_problem_test.cc

@@ -64,13 +64,16 @@ class QuadraticTestFunction : public ceres::FirstOrderFunction {
 
 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(),
+  ceres::GradientProblem problem(std::make_unique<QuadraticTestFunction>(),
-                                 new EuclideanManifold<1>);
+                                 std::make_unique<EuclideanManifold<1>>());
   double x = 7.0;
   double cost = 0;
   problem.Evaluate(&x, &cost, nullptr);
@@ -78,7 +81,7 @@ TEST(GradientProblem, EvaluationWithManifoldAndNoGradient) {
 }
 
 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, EvaluationWithoutManifoldAndWithGradient) {
 }
 
 TEST(GradientProblem, EvaluationWithManifoldAndWithGradient) {
-  ceres::GradientProblem problem(new QuadraticTestFunction(),
+  ceres::GradientProblem problem(std::make_unique<QuadraticTestFunction>(),
-                                 new EuclideanManifold<1>);
+                                 std::make_unique<EuclideanManifold<1>>());
   double x = 7.0;
   double cost = 0;
   double gradient = 0;

internal/ceres/line_search_minimizer_test.cc

@@ -52,7 +52,8 @@ class QuadraticFirstOrderFunction : public ceres::FirstOrderFunction {
 
 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);

internal/ceres/numeric_diff_first_order_function_test.cc

@@ -52,8 +52,7 @@ class QuadraticCostFunctor {
 
 TEST(NumericDiffFirstOrderFunction, BilinearDifferentiationTestStatic) {
   auto function = std::make_unique<
-      NumericDiffFirstOrderFunction<QuadraticCostFunctor, CENTRAL, 4>>(
+      NumericDiffFirstOrderFunction<QuadraticCostFunctor, CENTRAL, 4>>(1.0);
-      new QuadraticCostFunctor(1.0));
 
   double parameters[4] = {1.0, 2.0, 3.0, 4.0};
   double gradient[4];
@@ -77,7 +76,7 @@ TEST(NumericDiffFirstOrderFunction, BilinearDifferentiationTestStatic) {
 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];