From bcbaff6656070f9d81d41dac664cc765c7962f22 Mon Sep 17 00:00:00 2001 From: Sameer Agarwal Date: Tue, 17 Mar 2026 14:36:58 -0700 Subject: [PATCH] [modernize] Modernize AutoDiff and NumericDiff CostFunctions to C++17 Modernize the construction and initialization logic of AutoDiffCostFunction, NumericDiffCostFunction, AutoDiffFirstOrderFunction, NumericDiffFirstOrderFunction, DynamicAutoDiffCostFunction, DynamicNumericDiffCostFunction, and CostFunctionToFunctor by utilizing C++17 features: - Simplify constructor delegation using if constexpr, removing the need for internal InitTag structures. - Use std::is_constructible_v for cleaner SFINAE in variadic constructors. - Add static_assert to catch improper usage of DYNAMIC residuals/parameters at compile-time. - Add Ownership support to AutoDiffFirstOrderFunction, DynamicAutoDiffCostFunction, and DynamicNumericDiffCostFunction. - Simplify CostFunctionToFunctor::operator() using variadic parameter packs. - Use this->num_residuals() for consistent and clear access to base class state. - Minor code quality improvements and removal of redundant template parameters. Change-Id: Id760a26ba2ceca4fb2939ef7632f2f1d9cc430c9 --- include/ceres/autodiff_cost_function.h | 4 +- include/ceres/autodiff_first_order_function.h | 48 ++++++++++++++----- include/ceres/cost_function_to_functor.h | 29 +++-------- .../ceres/dynamic_autodiff_cost_function.h | 26 ++++++---- .../dynamic_numeric_diff_cost_function.h | 35 +++++++------- include/ceres/numeric_diff_cost_function.h | 4 +- 6 files changed, 81 insertions(+), 65 deletions(-) diff --git a/include/ceres/autodiff_cost_function.h b/include/ceres/autodiff_cost_function.h index 878b2ecaa..4ca8c4e6b 100644 --- a/include/ceres/autodiff_cost_function.h +++ b/include/ceres/autodiff_cost_function.h @@ -232,7 +232,7 @@ class AutoDiffCostFunction final return internal::AutoDifferentiate( *functor_, parameters, - SizedCostFunction::num_residuals(), + this->num_residuals(), residuals, jacobians); }; @@ -256,7 +256,7 @@ class AutoDiffCostFunction final "residuals is set to ceres::DYNAMIC."); if constexpr (InitTag::value == DYNAMIC_INIT) { - SizedCostFunction::set_num_residuals(num_residuals); + this->set_num_residuals(num_residuals); } } diff --git a/include/ceres/autodiff_first_order_function.h b/include/ceres/autodiff_first_order_function.h index 0e7def381..d29c8d6b1 100644 --- a/include/ceres/autodiff_first_order_function.h +++ b/include/ceres/autodiff_first_order_function.h @@ -91,7 +91,7 @@ namespace ceres { // // FirstOrderFunction* function = // new AutoDiffFirstOrderFunction( -// std::make_unique(1.0))); +// std::make_unique(1.0)); // // In the instantiation above, the template parameters following // "QuadraticCostFunctor", "4", describe the functor as computing a @@ -105,6 +105,32 @@ namespace ceres { template class AutoDiffFirstOrderFunction final : public FirstOrderFunction { public: + // Takes ownership of functor by default. + explicit AutoDiffFirstOrderFunction( + std::unique_ptr functor) + : AutoDiffFirstOrderFunction(std::move(functor), TAKE_OWNERSHIP) { + static_assert(kNumParameters > 0, "kNumParameters must be positive"); + } + + // Constructs the FirstOrderFunctor on the heap and takes the ownership. + template >> + explicit AutoDiffFirstOrderFunction(Args&&... args) + // NOTE We explicitly use direct initialization using parentheses instead + // of uniform initialization using braces to avoid narrowing conversion + // warnings. + : AutoDiffFirstOrderFunction( + std::make_unique(std::forward(args)...), + TAKE_OWNERSHIP) {} + + explicit AutoDiffFirstOrderFunction(FirstOrderFunctor* functor, + Ownership ownership = TAKE_OWNERSHIP) + : AutoDiffFirstOrderFunction(std::unique_ptr(functor), + ownership) { + static_assert(kNumParameters > 0, "kNumParameters must be positive"); + } + AutoDiffFirstOrderFunction(const AutoDiffFirstOrderFunction&) = delete; AutoDiffFirstOrderFunction& operator=(const AutoDiffFirstOrderFunction&) = delete; @@ -113,19 +139,12 @@ class AutoDiffFirstOrderFunction final : public FirstOrderFunction { AutoDiffFirstOrderFunction& operator=( AutoDiffFirstOrderFunction&& other) noexcept = default; - explicit AutoDiffFirstOrderFunction( - std::unique_ptr functor) - : functor_(std::move(functor)) { - static_assert(kNumParameters > 0, "kNumParameters must be positive"); + ~AutoDiffFirstOrderFunction() override { + if (ownership_ == DO_NOT_TAKE_OWNERSHIP) { + functor_.release(); + } } - template >* = nullptr> - explicit AutoDiffFirstOrderFunction(Args&&... args) - : AutoDiffFirstOrderFunction{ - std::make_unique(std::forward(args)...)} {} - bool Evaluate(const double* const parameters, double* cost, double* gradient) const override { @@ -159,7 +178,12 @@ class AutoDiffFirstOrderFunction final : public FirstOrderFunction { const FirstOrderFunctor& functor() const { return *functor_; } private: + AutoDiffFirstOrderFunction(std::unique_ptr functor, + Ownership ownership) + : functor_(std::move(functor)), ownership_(ownership) {} + std::unique_ptr functor_; + Ownership ownership_; }; } // namespace ceres diff --git a/include/ceres/cost_function_to_functor.h b/include/ceres/cost_function_to_functor.h index a2ca6d85d..9505159da 100644 --- a/include/ceres/cost_function_to_functor.h +++ b/include/ceres/cost_function_to_functor.h @@ -134,37 +134,20 @@ class CostFunctionToFunctor { template bool operator()(const T* p1, Ts*... ps) const { - // Add one because of residual block. static_assert(sizeof...(Ts) + 1 == ParameterDims::kNumParameterBlocks + 1, "Invalid number of parameter blocks specified."); - auto params = std::make_tuple(p1, ps...); - - // Extract residual pointer from params. The residual pointer is the - // last pointer. - constexpr int kResidualIndex = ParameterDims::kNumParameterBlocks; - T* residuals = std::get(params); - - // Extract parameter block pointers from params. - using Indices = - std::make_integer_sequence; - std::array parameter_blocks = - GetParameterPointers(params, Indices()); - - return cost_functor_(parameter_blocks.data(), residuals); + return [this](const auto&... pointers) { + const std::array + params{{pointers...}}; + T* residuals = const_cast(params[ParameterDims::kNumParameterBlocks]); + return cost_functor_(params.data(), residuals); + }(p1, ps...); } private: using ParameterDims = internal::StaticParameterDims; - template - static std::array - GetParameterPointers(const Tuple& paramPointers, - std::integer_sequence) { - return std::array{ - {std::get(paramPointers)...}}; - } - DynamicCostFunctionToFunctor cost_functor_; }; diff --git a/include/ceres/dynamic_autodiff_cost_function.h b/include/ceres/dynamic_autodiff_cost_function.h index 7efdad0e5..05b13bc8d 100644 --- a/include/ceres/dynamic_autodiff_cost_function.h +++ b/include/ceres/dynamic_autodiff_cost_function.h @@ -79,25 +79,31 @@ namespace ceres { template class DynamicAutoDiffCostFunction final : public DynamicCostFunction { public: + // Takes ownership of functor by default. + explicit DynamicAutoDiffCostFunction(std::unique_ptr functor) + : DynamicAutoDiffCostFunction(std::move(functor), TAKE_OWNERSHIP) { + static_assert(Stride > 0, "Stride must be positive"); + } + // Constructs the CostFunctor on the heap and takes the ownership. - template >* = - nullptr> + template >> explicit DynamicAutoDiffCostFunction(Args&&... args) // NOTE We explicitly use direct initialization using parentheses instead // of uniform initialization using braces to avoid narrowing conversion // warnings. - : DynamicAutoDiffCostFunction{ - std::make_unique(std::forward(args)...)} {} + : DynamicAutoDiffCostFunction( + std::make_unique(std::forward(args)...), + TAKE_OWNERSHIP) {} // Takes ownership by default. explicit DynamicAutoDiffCostFunction(CostFunctor* functor, Ownership ownership = TAKE_OWNERSHIP) - : DynamicAutoDiffCostFunction{std::unique_ptr{functor}, - ownership} {} - - explicit DynamicAutoDiffCostFunction(std::unique_ptr functor) - : DynamicAutoDiffCostFunction{std::move(functor), TAKE_OWNERSHIP} {} + : DynamicAutoDiffCostFunction(std::unique_ptr(functor), + ownership) { + static_assert(Stride > 0, "Stride must be positive"); + } DynamicAutoDiffCostFunction(const DynamicAutoDiffCostFunction& other) = delete; diff --git a/include/ceres/dynamic_numeric_diff_cost_function.h b/include/ceres/dynamic_numeric_diff_cost_function.h index 2cd0e9c99..55b79136a 100644 --- a/include/ceres/dynamic_numeric_diff_cost_function.h +++ b/include/ceres/dynamic_numeric_diff_cost_function.h @@ -72,36 +72,39 @@ namespace ceres { // also specify the sizes after creating the // DynamicNumericDiffCostFunction. For example: // -// DynamicAutoDiffCostFunction cost_function; +// DynamicNumericDiffCostFunction cost_function; // cost_function.AddParameterBlock(5); // cost_function.AddParameterBlock(10); // cost_function.SetNumResiduals(21); template class DynamicNumericDiffCostFunction final : public DynamicCostFunction { public: - explicit DynamicNumericDiffCostFunction( - const CostFunctor* functor, - Ownership ownership = TAKE_OWNERSHIP, - const NumericDiffOptions& options = NumericDiffOptions()) - : DynamicNumericDiffCostFunction{ - std::unique_ptr{functor}, ownership, options} {} - + // Takes ownership of functor by default. explicit DynamicNumericDiffCostFunction( std::unique_ptr functor, const NumericDiffOptions& options = NumericDiffOptions()) - : DynamicNumericDiffCostFunction{ - std::move(functor), TAKE_OWNERSHIP, options} {} + : DynamicNumericDiffCostFunction( + std::move(functor), TAKE_OWNERSHIP, options) {} // Constructs the CostFunctor on the heap and takes the ownership. template >* = - nullptr> + typename = std::enable_if_t>> explicit DynamicNumericDiffCostFunction(Args&&... args) // NOTE We explicitly use direct initialization using parentheses instead // of uniform initialization using braces to avoid narrowing conversion // warnings. - : DynamicNumericDiffCostFunction{ - std::make_unique(std::forward(args)...)} {} + : DynamicNumericDiffCostFunction( + std::make_unique(std::forward(args)...), + TAKE_OWNERSHIP, + NumericDiffOptions()) {} + + explicit DynamicNumericDiffCostFunction( + const CostFunctor* functor, + Ownership ownership = TAKE_OWNERSHIP, + const NumericDiffOptions& options = NumericDiffOptions()) + : DynamicNumericDiffCostFunction( + std::unique_ptr(functor), ownership, options) {} DynamicNumericDiffCostFunction(const DynamicNumericDiffCostFunction&) = delete; @@ -113,7 +116,7 @@ class DynamicNumericDiffCostFunction final : public DynamicCostFunction { DynamicNumericDiffCostFunction&& other) noexcept = default; ~DynamicNumericDiffCostFunction() override { - if (ownership_ != TAKE_OWNERSHIP) { + if (ownership_ == DO_NOT_TAKE_OWNERSHIP) { functor_.release(); } } @@ -220,4 +223,4 @@ DynamicNumericDiffCostFunction(std::unique_ptr functor, } // namespace ceres -#endif // CERES_PUBLIC_DYNAMIC_AUTODIFF_COST_FUNCTION_H_ +#endif // CERES_PUBLIC_DYNAMIC_NUMERIC_DIFF_COST_FUNCTION_H_ diff --git a/include/ceres/numeric_diff_cost_function.h b/include/ceres/numeric_diff_cost_function.h index 47e4af4e3..5b102aed7 100644 --- a/include/ceres/numeric_diff_cost_function.h +++ b/include/ceres/numeric_diff_cost_function.h @@ -263,7 +263,7 @@ class NumericDiffCostFunction final functor_.get(), residuals, options_, - SizedCostFunction::num_residuals(), + this->num_residuals(), parameters_reference_copy.data(), jacobians); @@ -279,7 +279,7 @@ class NumericDiffCostFunction final const NumericDiffOptions& options) : functor_(std::move(functor)), ownership_(ownership), options_(options) { if constexpr (kNumResiduals == DYNAMIC) { - SizedCostFunction::set_num_residuals(num_residuals); + this->set_num_residuals(num_residuals); } }