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];