From 6f5cdcc9ce2d3cd9be52d5e8cdf8ab71aec0497d Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Thu, 14 Nov 2024 12:52:52 -0500 Subject: [PATCH 1/3] Make fewer copies of arrays --- ql/math/optimization/levenbergmarquardt.cpp | 28 ++++++++++----------- ql/math/optimization/levenbergmarquardt.hpp | 14 +++-------- ql/math/optimization/problem.hpp | 4 +-- ql/termstructures/globalbootstrap.hpp | 9 ++----- 4 files changed, 20 insertions(+), 35 deletions(-) diff --git a/ql/math/optimization/levenbergmarquardt.cpp b/ql/math/optimization/levenbergmarquardt.cpp index 23015289fd2..fff0176de1e 100644 --- a/ql/math/optimization/levenbergmarquardt.cpp +++ b/ql/math/optimization/levenbergmarquardt.cpp @@ -36,17 +36,16 @@ namespace QuantLib { EndCriteria::Type LevenbergMarquardt::minimize(Problem& P, const EndCriteria& endCriteria) { P.reset(); - Array x_ = P.currentValue(); + const Array& initX = P.currentValue(); currentProblem_ = &P; - initCostValues_ = P.costFunction().values(x_); + initCostValues_ = P.costFunction().values(initX); int m = initCostValues_.size(); - int n = x_.size(); - if(useCostFunctionsJacobian_) { + int n = initX.size(); + if (useCostFunctionsJacobian_) { initJacobian_ = Matrix(m,n); - P.costFunction().jacobian(initJacobian_, x_); + P.costFunction().jacobian(initJacobian_, initX); } - std::unique_ptr xx(new Real[n]); - std::copy(x_.begin(), x_.end(), xx.get()); + Array xx = initX; std::unique_ptr fvec(new Real[m]); std::unique_ptr diag(new Real[n]); int mode = 1; @@ -81,15 +80,15 @@ namespace QuantLib { // in n variables by the Levenberg-Marquardt algorithm. MINPACK::LmdifCostFunction lmdifCostFunction = [this](const auto m, const auto n, const auto x, const auto fvec, const auto iflag) { - this->fcn(m, n, x, fvec, iflag); + this->fcn(m, n, x, fvec); }; MINPACK::LmdifCostFunction lmdifJacFunction = useCostFunctionsJacobian_ ? [this](const auto m, const auto n, const auto x, const auto fjac, const auto iflag) { - this->jacFcn(m, n, x, fjac, iflag); + this->jacFcn(m, n, x, fjac); } : MINPACK::LmdifCostFunction(); - MINPACK::lmdif(m, n, xx.get(), fvec.get(), + MINPACK::lmdif(m, n, xx.begin(), fvec.get(), endCriteria.functionEpsilon(), xtol_, gtol_, @@ -132,14 +131,13 @@ namespace QuantLib { QL_FAIL("unknown MINPACK result: " << info); } // set problem - std::copy(xx.get(), xx.get()+n, x_.begin()); - P.setCurrentValue(x_); - P.setFunctionValue(P.costFunction().value(x_)); + P.setCurrentValue(std::move(xx)); + P.setFunctionValue(P.costFunction().value(P.currentValue())); return ecType; } - void LevenbergMarquardt::fcn(int, int n, Real* x, Real* fvec, int*) { + void LevenbergMarquardt::fcn(int, int n, Real* x, Real* fvec) { Array xt(n); std::copy(x, x+n, xt.begin()); // constraint handling needs some improvement in the future: @@ -152,7 +150,7 @@ namespace QuantLib { } } - void LevenbergMarquardt::jacFcn(int m, int n, Real* x, Real* fjac, int*) { + void LevenbergMarquardt::jacFcn(int m, int n, Real* x, Real* fjac) { Array xt(n); std::copy(x, x+n, xt.begin()); // constraint handling needs some improvement in the future: diff --git a/ql/math/optimization/levenbergmarquardt.hpp b/ql/math/optimization/levenbergmarquardt.hpp index 6e60f1f44bc..8dcaa1acf9f 100644 --- a/ql/math/optimization/levenbergmarquardt.hpp +++ b/ql/math/optimization/levenbergmarquardt.hpp @@ -55,23 +55,15 @@ namespace QuantLib { EndCriteria::Type minimize(Problem& P, const EndCriteria& endCriteria) override; - void fcn(int m, - int n, - Real* x, - Real* fvec, - int* iflag); - void jacFcn(int m, - int n, - Real* x, - Real* fjac, - int* iflag); - /*! \deprecated Don't use this method; inspect the result of minimize instead. Deprecated in version 1.36. */ [[deprecated("Don't use this method; inspect the result of minimize instead")]] virtual Integer getInfo() const { return info_; } private: + void fcn(int m, int n, Real* x, Real* fvec); + void jacFcn(int m, int n, Real* x, Real* fjac); + Problem* currentProblem_; Array initCostValues_; Matrix initJacobian_; diff --git a/ql/math/optimization/problem.hpp b/ql/math/optimization/problem.hpp index ea13f461b42..2b5ae15b595 100644 --- a/ql/math/optimization/problem.hpp +++ b/ql/math/optimization/problem.hpp @@ -73,8 +73,8 @@ namespace QuantLib { //! Cost function CostFunction& costFunction() const { return costFunction_; } - void setCurrentValue(const Array& currentValue) { - currentValue_=currentValue; + void setCurrentValue(Array currentValue) { + currentValue_ = std::move(currentValue); } //! current value of the local minimum diff --git a/ql/termstructures/globalbootstrap.hpp b/ql/termstructures/globalbootstrap.hpp index 4d39501ee37..2dc55352da1 100644 --- a/ql/termstructures/globalbootstrap.hpp +++ b/ql/termstructures/globalbootstrap.hpp @@ -272,12 +272,6 @@ template void GlobalBootstrap::calculate() const { return std::tan((y - lowerBounds_[i]) * M_PI / (upperBounds_[i] - lowerBounds_[i]) - M_PI_2); } - Real value(const Array& x) const override { - Array v = values(x); - std::transform(v.begin(), v.end(), v.begin(), [](Real x) -> Real { return x*x; }); - return std::sqrt(std::accumulate(v.begin(), v.end(), Real(0.0)) / static_cast(v.size())); - } - Array values(const Array& x) const override { for (Size i = 0; i < x.size(); ++i) { Traits::updateGuess(ts_->data_, transformDirect(x[i], i), i + 1); @@ -304,7 +298,8 @@ template void GlobalBootstrap::calculate() const { Curve *ts_; const std::vector lowerBounds_, upperBounds_; }; - TargetFunction cost(firstHelper_, numberHelpers_, additionalErrors_, ts_, lowerBounds, upperBounds); + TargetFunction cost(firstHelper_, numberHelpers_, additionalErrors_, ts_, + std::move(lowerBounds), std::move(upperBounds)); // setup guess Array guess(numberBounds); From 760fb259fd004ca6b92c764338c91e6e1c9e696a Mon Sep 17 00:00:00 2001 From: Eugene Toder Date: Mon, 18 Nov 2024 11:38:34 -0500 Subject: [PATCH 2/3] Address review comments --- ql/math/optimization/levenbergmarquardt.hpp | 6 +++++ ql/termstructures/globalbootstrap.hpp | 30 ++++++++++++--------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/ql/math/optimization/levenbergmarquardt.hpp b/ql/math/optimization/levenbergmarquardt.hpp index 8dcaa1acf9f..b18c2298179 100644 --- a/ql/math/optimization/levenbergmarquardt.hpp +++ b/ql/math/optimization/levenbergmarquardt.hpp @@ -60,6 +60,12 @@ namespace QuantLib { */ [[deprecated("Don't use this method; inspect the result of minimize instead")]] virtual Integer getInfo() const { return info_; } + + [[deprecated("Don't use this method; it is private")]] + void fcn(int m, int n, Real* x, Real* fvec, int*) { fcn(m, n, x, fvec); } + + [[deprecated("Don't use this method; it is private")]] + void jacFcn(int m, int n, Real* x, Real* fjac, int*) { jacFcn(m, n, x, fjac); } private: void fcn(int m, int n, Real* x, Real* fvec); void jacFcn(int m, int n, Real* x, Real* fjac); diff --git a/ql/termstructures/globalbootstrap.hpp b/ql/termstructures/globalbootstrap.hpp index 2dc55352da1..cecbe9ddd37 100644 --- a/ql/termstructures/globalbootstrap.hpp +++ b/ql/termstructures/globalbootstrap.hpp @@ -254,15 +254,21 @@ template void GlobalBootstrap::calculate() const { // setup cost function class TargetFunction : public CostFunction { public: - TargetFunction(const Size firstHelper, - const Size numberHelpers, - std::function additionalErrors, + // NOTE: TargetFunction retains passed pointers, so they must point to objects + // that outlive the function. We use pointers instead of const refs to avoid + // accidental binding to temporaries. + TargetFunction(Size firstHelper, + Size numberHelpers, + const std::function* additionalErrors, Curve* ts, - std::vector lowerBounds, - std::vector upperBounds) + const std::vector* lowerBounds, + const std::vector* upperBounds) : firstHelper_(firstHelper), numberHelpers_(numberHelpers), - additionalErrors_(std::move(additionalErrors)), ts_(ts), - lowerBounds_(std::move(lowerBounds)), upperBounds_(std::move(upperBounds)) {} + additionalErrors_(*additionalErrors), ts_(ts), + lowerBounds_(*lowerBounds), upperBounds_(*upperBounds) { + QL_REQUIRE(additionalErrors != nullptr, "null additionalErrors"); + QL_REQUIRE(lowerBounds != nullptr && upperBounds != nullptr, "null bounds"); + } Real transformDirect(const Real x, const Size i) const { return (std::atan(x) + M_PI_2) / M_PI * (upperBounds_[i] - lowerBounds_[i]) + lowerBounds_[i]; @@ -294,12 +300,12 @@ template void GlobalBootstrap::calculate() const { private: Size firstHelper_, numberHelpers_; - std::function additionalErrors_; - Curve *ts_; - const std::vector lowerBounds_, upperBounds_; + const std::function& additionalErrors_; + Curve* ts_; + const std::vector &lowerBounds_, &upperBounds_; }; - TargetFunction cost(firstHelper_, numberHelpers_, additionalErrors_, ts_, - std::move(lowerBounds), std::move(upperBounds)); + TargetFunction cost(firstHelper_, numberHelpers_, &additionalErrors_, ts_, + &lowerBounds, &upperBounds); // setup guess Array guess(numberBounds); From 0fb7e68d8059ac3d1b996c37ba8bbcd046b87bb6 Mon Sep 17 00:00:00 2001 From: Luigi Ballabio Date: Tue, 19 Nov 2024 09:52:08 +0100 Subject: [PATCH 3/3] Document deprecations in Doxygen reference --- ql/math/optimization/levenbergmarquardt.hpp | 10 ++++++++-- ql/termstructures/globalbootstrap.hpp | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/ql/math/optimization/levenbergmarquardt.hpp b/ql/math/optimization/levenbergmarquardt.hpp index b18c2298179..b402770fe85 100644 --- a/ql/math/optimization/levenbergmarquardt.hpp +++ b/ql/math/optimization/levenbergmarquardt.hpp @@ -61,10 +61,16 @@ namespace QuantLib { [[deprecated("Don't use this method; inspect the result of minimize instead")]] virtual Integer getInfo() const { return info_; } - [[deprecated("Don't use this method; it is private")]] + /*! \deprecated Don't use this method; it is for internal use. + Deprecated in version 1.37. + */ + [[deprecated("Don't use this method; it is for internal use")]] void fcn(int m, int n, Real* x, Real* fvec, int*) { fcn(m, n, x, fvec); } - [[deprecated("Don't use this method; it is private")]] + /*! \deprecated Don't use this method; it is for internal use. + Deprecated in version 1.37. + */ + [[deprecated("Don't use this method; it is for internal use")]] void jacFcn(int m, int n, Real* x, Real* fjac, int*) { jacFcn(m, n, x, fjac); } private: void fcn(int m, int n, Real* x, Real* fvec); diff --git a/ql/termstructures/globalbootstrap.hpp b/ql/termstructures/globalbootstrap.hpp index cecbe9ddd37..a37d18b4225 100644 --- a/ql/termstructures/globalbootstrap.hpp +++ b/ql/termstructures/globalbootstrap.hpp @@ -255,8 +255,8 @@ template void GlobalBootstrap::calculate() const { class TargetFunction : public CostFunction { public: // NOTE: TargetFunction retains passed pointers, so they must point to objects - // that outlive the function. We use pointers instead of const refs to avoid - // accidental binding to temporaries. + // that outlive the call to optimizer->minimize(). We use pointers instead of + // const refs to avoid accidental binding to temporaries. TargetFunction(Size firstHelper, Size numberHelpers, const std::function* additionalErrors,