diff --git a/roofit/histfactory/test/testHistFactory.cxx b/roofit/histfactory/test/testHistFactory.cxx index ec0aee3fc4dcf..1502dca7fec7f 100644 --- a/roofit/histfactory/test/testHistFactory.cxx +++ b/roofit/histfactory/test/testHistFactory.cxx @@ -590,82 +590,67 @@ TEST_P(HFFixtureFit, Fit) auto mc = dynamic_cast(ws->obj("ModelConfig")); ASSERT_NE(mc, nullptr); - // This tests both correct pre-caching of constant terms and (if false) that all doEval() are correct. - for (bool constTermOptimization : {true, false}) { + std::unique_ptr pars(simPdf->getParameters(*data)); + // Kick parameters: + for (auto par : *pars) { + auto real = dynamic_cast(par); + if (real && !real->isConstant()) + real->setVal(real->getVal() * 0.95); + } + if (makeModelMode == MakeModelMode::StatSyst) { + auto poi = dynamic_cast(pars->find("SigXsecOverSM")); + ASSERT_NE(poi, nullptr); + poi->setVal(2.); + poi->setConstant(); + } - // constTermOptimization makes only sense in the legacy backend - if (constTermOptimization && evalBackend != RooFit::EvalBackend::Legacy()) { - continue; - } - SCOPED_TRACE(constTermOptimization ? "const term optimisation" : "No const term optimisation"); - - // Stop if one of the previous runs had a failure to keep the terminal clean. - if (HasFailure()) - break; - - std::unique_ptr pars(simPdf->getParameters(*data)); - // Kick parameters: - for (auto par : *pars) { - auto real = dynamic_cast(par); - if (real && !real->isConstant()) - real->setVal(real->getVal() * 0.95); - } - if (makeModelMode == MakeModelMode::StatSyst) { - auto poi = dynamic_cast(pars->find("SigXsecOverSM")); - ASSERT_NE(poi, nullptr); - poi->setVal(2.); - poi->setConstant(); + using namespace RooFit; + std::unique_ptr fitResult{simPdf->fitTo( + *data, evalBackend, GlobalObservables(*mc->GetGlobalObservables()), Save(), PrintLevel(verbose ? 1 : -1))}; + ASSERT_NE(fitResult, nullptr); + if (verbose) + fitResult->Print("v"); + EXPECT_EQ(fitResult->status(), 0); + + auto checkParam = [&](const std::string ¶m, double target, double absPrecision = 1.e-2) { + auto par = dynamic_cast(fitResult->floatParsFinal().find(param.c_str())); + if (!par) { + // Parameter was constant in this fit + par = dynamic_cast(fitResult->constPars().find(param.c_str())); + ASSERT_NE(par, nullptr) << param; + EXPECT_DOUBLE_EQ(par->getVal(), target) << "Constant parameter " << param << " is off target."; + } else { + EXPECT_NEAR(par->getVal(), target, par->getError()) + << "Parameter " << param << " close to target " << target << " within uncertainty"; + EXPECT_NEAR(par->getVal(), target, absPrecision) << "Parameter " << param << " close to target " << target; } + }; - using namespace RooFit; - std::unique_ptr fitResult{simPdf->fitTo(*data, evalBackend, Optimize(constTermOptimization), - GlobalObservables(*mc->GetGlobalObservables()), Save(), - PrintLevel(verbose ? 1 : -1))}; - ASSERT_NE(fitResult, nullptr); - if (verbose) - fitResult->Print("v"); - EXPECT_EQ(fitResult->status(), 0); - - auto checkParam = [&](const std::string ¶m, double target, double absPrecision = 1.e-2) { - auto par = dynamic_cast(fitResult->floatParsFinal().find(param.c_str())); - if (!par) { - // Parameter was constant in this fit - par = dynamic_cast(fitResult->constPars().find(param.c_str())); - ASSERT_NE(par, nullptr) << param; - EXPECT_DOUBLE_EQ(par->getVal(), target) << "Constant parameter " << param << " is off target."; - } else { - EXPECT_NEAR(par->getVal(), target, par->getError()) - << "Parameter " << param << " close to target " << target << " within uncertainty"; - EXPECT_NEAR(par->getVal(), target, absPrecision) << "Parameter " << param << " close to target " << target; - } - }; - - if (makeModelMode == MakeModelMode::OverallSyst) { - // Model is set up such that background scale factors should be close to 1, and signal == 2 - checkParam("SigXsecOverSM", 2.); - checkParam("alpha_syst2", 0.); - checkParam("alpha_syst3", 0.); - checkParam("alpha_syst4", 0.); - checkParam("gamma_stat_channel1_bin_0", 1.); - checkParam("gamma_stat_channel1_bin_1", 1.); - } else if (makeModelMode == MakeModelMode::HistoSyst) { - // Model is set up with a -1 sigma pull on the signal shape parameter. - checkParam("SigXsecOverSM", 2., 1.1E-1); // Higher tolerance: Expect a pull due to shape syst. - checkParam("gamma_stat_channel1_bin_0", 1.); - checkParam("gamma_stat_channel1_bin_1", 1.); - checkParam("alpha_SignalShape", -0.9, 5.E-2); // Pull slightly lower than 1 because of constraint term - } else if (makeModelMode == MakeModelMode::StatSyst) { - // Model is set up with a -1 sigma pull on the signal shape parameter. - checkParam("SigXsecOverSM", 2., 1.1E-1); // Higher tolerance: Expect a pull due to shape syst. - checkParam("gamma_stat_channel1_bin_0", 1.09); // This should be pulled - checkParam("gamma_stat_channel1_bin_1", 1.); - } else if (makeModelMode == MakeModelMode::ShapeSyst) { - // This should be pulled down - checkParam("gamma_background1Shape_bin_0", 0.8866, 0.03); - // This should be pulled up, but not so much because the free signal - // strength will fit the excess in this bin. - checkParam("gamma_background2Shape_bin_1", 1.0250, 0.03); - } + if (makeModelMode == MakeModelMode::OverallSyst) { + // Model is set up such that background scale factors should be close to 1, and signal == 2 + checkParam("SigXsecOverSM", 2.); + checkParam("alpha_syst2", 0.); + checkParam("alpha_syst3", 0.); + checkParam("alpha_syst4", 0.); + checkParam("gamma_stat_channel1_bin_0", 1.); + checkParam("gamma_stat_channel1_bin_1", 1.); + } else if (makeModelMode == MakeModelMode::HistoSyst) { + // Model is set up with a -1 sigma pull on the signal shape parameter. + checkParam("SigXsecOverSM", 2., 1.1E-1); // Higher tolerance: Expect a pull due to shape syst. + checkParam("gamma_stat_channel1_bin_0", 1.); + checkParam("gamma_stat_channel1_bin_1", 1.); + checkParam("alpha_SignalShape", -0.9, 5.E-2); // Pull slightly lower than 1 because of constraint term + } else if (makeModelMode == MakeModelMode::StatSyst) { + // Model is set up with a -1 sigma pull on the signal shape parameter. + checkParam("SigXsecOverSM", 2., 1.1E-1); // Higher tolerance: Expect a pull due to shape syst. + checkParam("gamma_stat_channel1_bin_0", 1.09); // This should be pulled + checkParam("gamma_stat_channel1_bin_1", 1.); + } else if (makeModelMode == MakeModelMode::ShapeSyst) { + // This should be pulled down + checkParam("gamma_background1Shape_bin_0", 0.8866, 0.03); + // This should be pulled up, but not so much because the free signal + // strength will fit the excess in this bin. + checkParam("gamma_background2Shape_bin_1", 1.0250, 0.03); } if (false) { diff --git a/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx b/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx index 6d2c5adb01e7d..d5bef85c7a366 100644 --- a/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx +++ b/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx @@ -479,7 +479,7 @@ std::unique_ptr PDFTest::runBatchFit(RooAbsPdf *pdf) MyTimer batchTimer("Fitting batch mode " + _name); std::unique_ptr result{pdf->fitTo(*_dataFit, RooFit::EvalBackend::Cpu(), RooFit::SumW2Error(false), - RooFit::Optimize(1), RooFit::PrintLevel(_printLevel), RooFit::Save(), + RooFit::PrintLevel(_printLevel), RooFit::Save(), _multiProcess > 0 ? RooFit::NumCPU(_multiProcess) : RooCmdArg())}; std::cout << batchTimer; EXPECT_NE(result, nullptr); diff --git a/roofit/roofit/test/vectorisedPDFs/testGaussBinned.cxx b/roofit/roofit/test/vectorisedPDFs/testGaussBinned.cxx index 976f0d3e0cb5c..4840edd7c412d 100644 --- a/roofit/roofit/test/vectorisedPDFs/testGaussBinned.cxx +++ b/roofit/roofit/test/vectorisedPDFs/testGaussBinned.cxx @@ -87,7 +87,7 @@ TEST_P(GaussBinnedFit, BatchFit) m.setVal(-1.); s.setVal(3.); MyTimer timer(evalBackend == EvalBackend::Value::Cpu ? "BatchBinned" : "ScalarBinned"); - gaus.fitTo(*dataHist, EvalBackend(evalBackend), PrintLevel(-1), Optimize(0)); + gaus.fitTo(*dataHist, EvalBackend(evalBackend), PrintLevel(-1)); timer.interval(); std::cout << timer << std::endl; EXPECT_NEAR(m.getVal(), 1., m.getError()); diff --git a/roofit/roofitcore/CMakeLists.txt b/roofit/roofitcore/CMakeLists.txt index a638e0267b2d0..c272de7e49b3a 100644 --- a/roofit/roofitcore/CMakeLists.txt +++ b/roofit/roofitcore/CMakeLists.txt @@ -436,7 +436,6 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitCore src/RooVectorDataStore.cxx src/RooWorkspace.cxx src/RooWrapperPdf.cxx - src/TestStatistics/ConstantTermsOptimizer.cxx src/TestStatistics/LikelihoodGradientWrapper.cxx src/TestStatistics/LikelihoodSerial.cxx src/TestStatistics/LikelihoodWrapper.cxx diff --git a/roofit/roofitcore/inc/RooAbsArg.h b/roofit/roofitcore/inc/RooAbsArg.h index 46c1829637e57..1a3f987e61c43 100644 --- a/roofit/roofitcore/inc/RooAbsArg.h +++ b/roofit/roofitcore/inc/RooAbsArg.h @@ -340,9 +340,6 @@ class RooAbsArg : public TNamed, public RooPrintable { bool findConstantNodes(const RooArgSet &observables, RooArgSet &cacheList); bool findConstantNodes(const RooArgSet &observables, RooArgSet &cacheList, RooLinkedList &processedNodes); - // constant term optimization - virtual void constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt = true); - virtual CacheMode canNodeBeCached() const { return Always; } virtual void setCacheAndTrackHints(RooArgSet & /*trackNodes*/) {}; diff --git a/roofit/roofitcore/inc/RooAbsData.h b/roofit/roofitcore/inc/RooAbsData.h index 7a8bd894d6828..33a8e4054935a 100644 --- a/roofit/roofitcore/inc/RooAbsData.h +++ b/roofit/roofitcore/inc/RooAbsData.h @@ -304,9 +304,6 @@ class RooAbsData : public TNamed, public RooPrintable { double corrcov(const RooRealVar& x, const RooRealVar& y, const char* cutSpec, const char* cutRange, bool corr) const ; RooFit::OwningPtr corrcovMatrix(const RooArgList& vars, const char* cutSpec, const char* cutRange, bool corr) const ; - virtual void optimizeReadingWithCaching(RooAbsArg& arg, const RooArgSet& cacheList, const RooArgSet& keepObsList) ; - bool allClientsCached(RooAbsArg*, const RooArgSet&) ; - struct PlotOpt { const char* cuts = ""; Option_t* drawOptions = "P"; @@ -341,11 +338,6 @@ class RooAbsData : public TNamed, public RooPrintable { // for access into copied dataset: friend class RooFit::TestStatistics::RooAbsL; - virtual void cacheArgs(const RooAbsArg* owner, RooArgSet& varSet, const RooArgSet* nset=nullptr, bool skipZeroWeights=false) ; - virtual void resetCache() ; - virtual void setArgStatus(const RooArgSet& set, bool active) ; - virtual void attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVars) ; - virtual std::unique_ptr reduceEng(const RooArgSet& varSubset, const RooFormulaVar* cutVar, const char* cutRange=nullptr, std::size_t nStart = 0, std::size_t = std::numeric_limits::max()) const = 0 ; diff --git a/roofit/roofitcore/inc/RooAbsDataStore.h b/roofit/roofitcore/inc/RooAbsDataStore.h index 6e78e357868cf..9a71a511876dd 100644 --- a/roofit/roofitcore/inc/RooAbsDataStore.h +++ b/roofit/roofitcore/inc/RooAbsDataStore.h @@ -121,15 +121,6 @@ class RooAbsDataStore : public TNamed, public RooPrintable { int defaultPrintContents(Option_t* /*opt*/) const override { return kName|kClassName|kArgs|kValue ; } - // Constant term optimizer interface - virtual void cacheArgs(const RooAbsArg* cacheOwner, RooArgSet& varSet, const RooArgSet* nset=nullptr, bool skipZeroWeights=false) = 0 ; - virtual const RooAbsArg* cacheOwner() = 0 ; - virtual void attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVars) = 0 ; - virtual void setArgStatus(const RooArgSet& set, bool active) = 0 ; - const RooArgSet& cachedVars() const { return _cachedVars ; } - virtual void resetCache() = 0 ; - virtual void recalculateCache(const RooArgSet* /*proj*/, Int_t /*firstEvent*/, Int_t /*lastEvent*/, Int_t /*stepSize*/, bool /* skipZeroWeights*/) {} ; - virtual void setDirtyProp(bool flag) { _doDirtyProp = flag ; } bool dirtyProp() const { return _doDirtyProp ; } @@ -142,8 +133,6 @@ class RooAbsDataStore : public TNamed, public RooPrintable { virtual void loadValues(const RooAbsDataStore *tds, const RooFormulaVar* select=nullptr, const char* rangeName=nullptr, std::size_t nStart=0, std::size_t nStop = std::numeric_limits::max()) = 0 ; - virtual void forceCacheUpdate() {} ; - protected: RooArgSet _vars; diff --git a/roofit/roofitcore/inc/RooCompositeDataStore.h b/roofit/roofitcore/inc/RooCompositeDataStore.h index a9026996e4aed..b720c9d2ff62c 100644 --- a/roofit/roofitcore/inc/RooCompositeDataStore.h +++ b/roofit/roofitcore/inc/RooCompositeDataStore.h @@ -89,20 +89,9 @@ class RooCompositeDataStore : public RooAbsDataStore { void attachBuffers(const RooArgSet& extObs) override ; void resetBuffers() override ; - // Constant term optimizer interface - void cacheArgs(const RooAbsArg* owner, RooArgSet& varSet, const RooArgSet* nset=nullptr, bool skipZeroWeights=false) override ; - const RooAbsArg* cacheOwner() override { return nullptr ; } - void setArgStatus(const RooArgSet& set, bool active) override ; - void resetCache() override ; - - void recalculateCache(const RooArgSet* /*proj*/, Int_t /*firstEvent*/, Int_t /*lastEvent*/, Int_t /*stepSize*/, bool /*skipZeroWeights*/) override ; - bool hasFilledCache() const override ; - void loadValues(const RooAbsDataStore *tds, const RooFormulaVar* select=nullptr, const char* rangeName=nullptr, std::size_t nStart=0, std::size_t nStop = std::numeric_limits::max()) override; - void forceCacheUpdate() override ; - RooAbsData::RealSpans getBatches(std::size_t first, std::size_t len) const override { //TODO std::cerr << "This functionality is not yet implemented for composite data stores." << std::endl; @@ -115,8 +104,6 @@ class RooCompositeDataStore : public RooAbsDataStore { protected: - void attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVars) override ; - std::map _dataMap ; RooCategory* _indexCat = nullptr; mutable RooAbsDataStore* _curStore = nullptr; ///print(os); } - /// The RooFit::Evaluator is dealing with constant terms itself. - void constOptimizeTestStatistic(ConstOpCode /*opcode*/, bool /*doAlsoTrackingOpt*/) override {} - bool hasGradient() const override; bool hasHessian() const override; diff --git a/roofit/roofitcore/inc/RooFit/TestStatistics/LikelihoodWrapper.h b/roofit/roofitcore/inc/RooFit/TestStatistics/LikelihoodWrapper.h index ebe07196b3502..3aecfa67095c1 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/LikelihoodWrapper.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/LikelihoodWrapper.h @@ -94,7 +94,6 @@ class LikelihoodWrapper { virtual void updateMinuitExternalParameterValues(const std::vector &minuit_external_x); // The following functions are necessary from MinuitFcnGrad to reach likelihood properties: - void constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt); double defaultErrorLevel() const; virtual std::string GetName() const; virtual std::string GetTitle() const; diff --git a/roofit/roofitcore/inc/RooFit/TestStatistics/RooAbsL.h b/roofit/roofitcore/inc/RooFit/TestStatistics/RooAbsL.h index 819d06b43816a..5446f63a48727 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/RooAbsL.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/RooAbsL.h @@ -109,12 +109,6 @@ class RooAbsL { // necessary from MinuitFcnGrad to reach likelihood properties: virtual std::unique_ptr getParameters(); - /// \brief Interface function signaling a request to perform constant term optimization. - /// - /// The default implementation takes no action other than to forward the calls to all servers. May be overridden in - /// likelihood classes without a cached dataset, like RooSubsidiaryL. - virtual void constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt); - virtual std::string GetName() const; virtual std::string GetTitle() const; virtual std::string GetInfo() const { return GetClassName() + "::" + pdf_->GetName(); } diff --git a/roofit/roofitcore/inc/RooFit/TestStatistics/RooSubsidiaryL.h b/roofit/roofitcore/inc/RooFit/TestStatistics/RooSubsidiaryL.h index 44c663ca33c12..83025f2f18b2d 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/RooSubsidiaryL.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/RooSubsidiaryL.h @@ -47,8 +47,6 @@ class RooSubsidiaryL : public RooAbsL { return 0; } - void constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override; - private: std::string parent_pdf_name_; RooArgList subsidiary_pdfs_{"subsidiary_pdfs"}; ///< Set of subsidiary PDF or "constraint" terms diff --git a/roofit/roofitcore/inc/RooFit/TestStatistics/RooSumL.h b/roofit/roofitcore/inc/RooFit/TestStatistics/RooSumL.h index cb70523ac2d52..dae99db4e38ed 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/RooSumL.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/RooSumL.h @@ -35,8 +35,6 @@ class RooSumL : public RooAbsL { // necessary only for legacy offsetting mode in LikelihoodWrapper; TODO: remove this if legacy mode is ever removed ROOT::Math::KahanSum getSubsidiaryValue(); - void constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override; - std::string GetClassName() const override { return "RooSumL"; } const std::vector> &GetComponents() const { return components_; }; diff --git a/roofit/roofitcore/inc/RooGlobalFunc.h b/roofit/roofitcore/inc/RooGlobalFunc.h index 1a8f22a138445..c4d869f5ef198 100644 --- a/roofit/roofitcore/inc/RooGlobalFunc.h +++ b/roofit/roofitcore/inc/RooGlobalFunc.h @@ -253,11 +253,6 @@ RooCmdArg IntegrateBins(double precision); // RooAbsPdf::fitTo arguments RooCmdArg PrefitDataFraction(double data_ratio = 0.0) ; -RooCmdArg Optimize(Int_t flag=2) -#ifndef ROOFIT_BUILDS_ITSELF - R__DEPRECATED(6, 42, "Please use the default \"cpu\" likelihood evaluation backend if you want all optimizations.") -#endif - ; class EvalBackend : public RooCmdArg { public: diff --git a/roofit/roofitcore/inc/RooMinimizer.h b/roofit/roofitcore/inc/RooMinimizer.h index fcf2dfe960a64..f07b39620a87a 100644 --- a/roofit/roofitcore/inc/RooMinimizer.h +++ b/roofit/roofitcore/inc/RooMinimizer.h @@ -145,11 +145,6 @@ class RooMinimizer : public TObject { void setPrintLevel(int newLevel); // Setters on _fcn - void optimizeConst(int flag) -#ifndef ROOFIT_BUILDS_ITSELF - R__DEPRECATED(6, 42, "Please use the default \"cpu\" likelihood evaluation backend if you want all optimizations.") -#endif - ; void setEvalErrorWall(bool flag) { _cfg.doEEWall = flag; } void setRecoverFromNaNStrength(double strength); void setOffsetting(bool flag); diff --git a/roofit/roofitcore/inc/RooTreeDataStore.h b/roofit/roofitcore/inc/RooTreeDataStore.h index 319a1506594df..bc3010c673682 100644 --- a/roofit/roofitcore/inc/RooTreeDataStore.h +++ b/roofit/roofitcore/inc/RooTreeDataStore.h @@ -108,12 +108,6 @@ class RooTreeDataStore : public RooAbsDataStore { TTree* tree() { return _tree ; } TTree const *tree() const { return _tree ; } - // Constant term optimizer interface - void cacheArgs(const RooAbsArg* owner, RooArgSet& varSet, const RooArgSet* nset=nullptr, bool skipZeroWeights=false) override ; - const RooAbsArg* cacheOwner() override { return _cacheOwner ; } - void setArgStatus(const RooArgSet& set, bool active) override ; - void resetCache() override ; - void loadValues(const TTree *t, const RooFormulaVar* select=nullptr, const char* rangeName=nullptr, Int_t nStart=0, Int_t nStop=2000000000) ; void loadValues(const RooAbsDataStore *tds, const RooFormulaVar* select=nullptr, const char* rangeName=nullptr, std::size_t nStart=0, std::size_t nStop = std::numeric_limits::max()) override; @@ -138,7 +132,6 @@ class RooTreeDataStore : public RooAbsDataStore { RooRealVar* weightVar(const RooArgSet& allVars, const char* wgtName=nullptr) ; void initialize(); - void attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVars) override ; // TTree Branch buffer size control void setBranchBufferSize(Int_t size) { _defTreeBufSize = size ; } @@ -150,8 +143,6 @@ class RooTreeDataStore : public RooAbsDataStore { void createTree(RooStringView name, RooStringView title) ; TTree *_tree = nullptr; // TTree holding the data points - TTree *_cacheTree = nullptr; ///::max()) override; void dump() override; @@ -184,9 +172,6 @@ class RooVectorDataStore : public RooAbsDataStore { void setDirtyProp(bool flag) override { _doDirtyProp = flag ; - if (_cache) { - _cache->setDirtyProp(flag) ; - } } const RooArgSet& row() { return _varsww ; } @@ -559,10 +544,6 @@ class RooVectorDataStore : public RooAbsDataStore { RealFullVector* addRealFull(RooAbsReal* real); - bool hasFilledCache() const override { return _cache ? true : false ; } - - void forceCacheUpdate() override; - private: RooArgSet _varsww ; RooRealVar* _wgtVar = nullptr; ///< Pointer to weight variable (if set) @@ -583,11 +564,6 @@ class RooVectorDataStore : public RooAbsDataStore { mutable ULong64_t _currentWeightIndex{0}; ///< - RooVectorDataStore* _cache = nullptr; /// minimize(RooAbsReal &pdf, RooAbsReal &nll, RooAbsD { MinimizerConfig cfg; cfg.recoverFromNaN = pc.getDouble("RecoverFromUndefinedRegions"); - cfg.optConst = pc.getInt("optConst"); cfg.verbose = pc.getInt("verbose"); cfg.doSave = pc.getInt("doSave"); cfg.doTimer = pc.getInt("doTimer"); @@ -548,8 +545,6 @@ std::unique_ptr minimize(RooAbsReal &pdf, RooAbsReal &nll, RooAbsD m.setMaxFunctionCalls(cfg.maxCalls); if (cfg.printLevel != 1) m.setPrintLevel(cfg.printLevel); - if (cfg.optConst) - m.optimizeConst(cfg.optConst); // Activate constant term optimization if (cfg.verbose) m.setVerbose(true); // Activate verbose options if (cfg.doTimer) @@ -584,8 +579,6 @@ std::unique_ptr minimize(RooAbsReal &pdf, RooAbsReal &nll, RooAbsD ret->setCovQual(corrCovQual); } - if (cfg.optConst) - m.optimizeConst(0); return ret; } @@ -610,7 +603,6 @@ std::unique_ptr createNLL(RooAbsPdf &pdf, RooAbsData &data, const Ro pc.defineInt("numcpu", "NumCPU", 0, 1); pc.defineInt("interleave", "NumCPU", 1, 0); pc.defineInt("verbose", "Verbose", 0, 0); - pc.defineInt("optConst", "Optimize", 0, 0); pc.defineInt("cloneData", "CloneData", 0, 2); pc.defineSet("projDepSet", "ProjectedObservables", 0, nullptr); pc.defineSet("cPars", "Constrain", 0, nullptr); @@ -672,13 +664,12 @@ std::unique_ptr createNLL(RooAbsPdf &pdf, RooAbsData &data, const Ro const bool ext = interpretExtendedCmdArg(pdf, pc.getInt("ext")); int splitRange = pc.getInt("splitRange"); - int optConst = pc.getInt("optConst"); int cloneData = pc.getInt("cloneData"); auto offset = static_cast(pc.getInt("doOffset")); - // If no explicit cloneData command is specified, cloneData is set to true if optimization is activated + // Ignored if (cloneData == 2) { - cloneData = optConst; + cloneData = 0; } if (pc.hasProcessed("Range")) { @@ -890,10 +881,6 @@ std::unique_ptr createNLL(RooAbsPdf &pdf, RooAbsData &data, const Ro nll->addOwnedComponents(std::move(orignll), std::move(constraintTerm)); } - if (optConst) { - nll->constOptimizeTestStatistic(RooAbsArg::Activate, optConst > 1); - } - if (offset == RooFit::OffsetMode::Initial) { nll->enableOffsetting(true); } @@ -1044,7 +1031,7 @@ std::unique_ptr fitTo(RooAbsReal &real, RooAbsData &data, const Ro nllCmdListString += ",OffsetLikelihood"; } } else { - auto createChi2DataHistCmdArgs = "Range,RangeWithName,NumCPU,Optimize,IntegrateBins,ProjectedObservables," + auto createChi2DataHistCmdArgs = "Range,RangeWithName,NumCPU,,IntegrateBins,ProjectedObservables," "AddCoefRange,SplitRange,DataError,Extended"; auto createChi2DataSetCmdArgs = "YVar,Integrate,RangeWithName,NumCPU,Verbose"; nllCmdListString += isDataHist ? createChi2DataHistCmdArgs : createChi2DataSetCmdArgs; diff --git a/roofit/roofitcore/src/RooAbsArg.cxx b/roofit/roofitcore/src/RooAbsArg.cxx index e5c948d99a422..2d17e07419930 100644 --- a/roofit/roofitcore/src/RooAbsArg.cxx +++ b/roofit/roofitcore/src/RooAbsArg.cxx @@ -1712,17 +1712,6 @@ bool RooAbsArg::findConstantNodes(const RooArgSet &observables, RooArgSet &cache return false; } -/// Interface function signaling a request to perform constant term -/// optimization. This default implementation takes no action other than to -/// forward the calls to all servers - -void RooAbsArg::constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - for (const auto server : _serverList) { - server->constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); - } -} - /// Change cache operation mode to given mode. If recurseAdirty /// is true, then a mode change to AlwaysDirty will automatically /// be propagated recursively to all client nodes diff --git a/roofit/roofitcore/src/RooAbsData.cxx b/roofit/roofitcore/src/RooAbsData.cxx index 830527a7cf9c5..c3480a26da668 100644 --- a/roofit/roofitcore/src/RooAbsData.cxx +++ b/roofit/roofitcore/src/RooAbsData.cxx @@ -185,7 +185,6 @@ void RooAbsData::initializeVars(RooArgSet const& vars) RooAbsData::RooAbsData(RooStringView name, RooStringView title, const RooArgSet& vars, RooAbsDataStore* dstore) : TNamed(name,title), _vars("Dataset Variables"), - _cachedVars("Cached Variables"), _dstore(dstore) { if (dynamic_cast(dstore)) { @@ -246,8 +245,7 @@ void RooAbsData::copyImpl(const RooAbsData &other, const char *newName) RooAbsData::RooAbsData(const RooAbsData &other, const char *newName) : TNamed{newName ? newName : other.GetName(), other.GetTitle()}, - RooPrintable{other}, - _cachedVars{"Cached Variables"} + RooPrintable{other} { copyImpl(other, newName); @@ -342,38 +340,6 @@ const RooArgSet* RooAbsData::get(Int_t index) const return _dstore->get(index) ; } -//////////////////////////////////////////////////////////////////////////////// -/// Internal method -- Cache given set of functions with data - -void RooAbsData::cacheArgs(const RooAbsArg* cacheOwner, RooArgSet& varSet, const RooArgSet* nset, bool skipZeroWeights) -{ - _dstore->cacheArgs(cacheOwner,varSet,nset,skipZeroWeights) ; -} - -//////////////////////////////////////////////////////////////////////////////// -/// Internal method -- Remove cached function values - -void RooAbsData::resetCache() -{ - _dstore->resetCache() ; - _cachedVars.removeAll() ; -} - -//////////////////////////////////////////////////////////////////////////////// -/// Internal method -- Attach dataset copied with cache contents to copied instances of functions - -void RooAbsData::attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVars) -{ - _dstore->attachCache(newOwner, cachedVars) ; -} - -//////////////////////////////////////////////////////////////////////////////// - -void RooAbsData::setArgStatus(const RooArgSet& set, bool active) -{ - _dstore->setArgStatus(set,active) ; -} - //////////////////////////////////////////////////////////////////////////////// /// Control propagation of dirty flags from observables in dataset @@ -2243,85 +2209,6 @@ bool RooAbsData::getRange(const RooAbsRealLValue& var, double& lowest, double& h return false ; } -//////////////////////////////////////////////////////////////////////////////// -/// Prepare dataset for use with cached constant terms listed in -/// 'cacheList' of expression 'arg'. Deactivate tree branches -/// for any dataset observable that is either not used at all, -/// or is used exclusively by cached branch nodes. - -void RooAbsData::optimizeReadingWithCaching(RooAbsArg& arg, const RooArgSet& cacheList, const RooArgSet& keepObsList) -{ - RooArgSet pruneSet ; - - // Add unused observables in this dataset to pruneSet - pruneSet.add(*get()) ; - std::unique_ptr usedObs{arg.getObservables(*this)}; - pruneSet.remove(*usedObs,true,true) ; - - // Add observables exclusively used to calculate cached observables to pruneSet - for(auto * var : *get()) { - if (allClientsCached(var,cacheList)) { - pruneSet.add(*var) ; - } - } - - - if (!pruneSet.empty()) { - - // Go over all used observables and check if any of them have parameterized - // ranges in terms of pruned observables. If so, remove those observable - // from the pruning list - for(auto const* rrv : dynamic_range_cast(*usedObs)) { - if (rrv && !rrv->getBinning().isShareable()) { - RooArgSet depObs ; - RooAbsReal* loFunc = rrv->getBinning().lowBoundFunc() ; - RooAbsReal* hiFunc = rrv->getBinning().highBoundFunc() ; - if (loFunc) { - loFunc->leafNodeServerList(&depObs,nullptr,true) ; - } - if (hiFunc) { - hiFunc->leafNodeServerList(&depObs,nullptr,true) ; - } - if (!depObs.empty()) { - pruneSet.remove(depObs,true,true) ; - } - } - } - } - - - // Remove all observables in keep list from prune list - pruneSet.remove(keepObsList,true,true) ; - - if (!pruneSet.empty()) { - - // Deactivate tree branches here - cxcoutI(Optimization) << "RooTreeData::optimizeReadingForTestStatistic(" << GetName() << "): Observables " << pruneSet - << " in dataset are either not used at all, orserving exclusively p.d.f nodes that are now cached, disabling reading of these observables for TTree" << std::endl ; - setArgStatus(pruneSet,false) ; - } -} - -//////////////////////////////////////////////////////////////////////////////// -/// Utility function that determines if all clients of object 'var' -/// appear in given list of cached nodes. - -bool RooAbsData::allClientsCached(RooAbsArg* var, const RooArgSet& cacheList) -{ - bool ret(true); - bool anyClient(false); - - for (const auto client : var->valueClients()) { - anyClient = true ; - if (!cacheList.find(client->GetName())) { - // If client is not cached recurse - ret &= allClientsCached(client,cacheList) ; - } - } - - return anyClient?ret:false ; -} - //////////////////////////////////////////////////////////////////////////////// void RooAbsData::attachBuffers(const RooArgSet& extObs) @@ -2392,13 +2279,6 @@ void RooAbsData::Draw(Option_t* option) if (_dstore) _dstore->Draw(option) ; } -//////////////////////////////////////////////////////////////////////////////// - -bool RooAbsData::hasFilledCache() const -{ - return _dstore->hasFilledCache() ; -} - //////////////////////////////////////////////////////////////////////////////// /// Return a pointer to the TTree which stores the data. Returns a nullpointer /// if vector-based storage is used. The RooAbsData remains owner of the tree. diff --git a/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx b/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx index 413cfda2b90b3..1e5ed00c84593 100644 --- a/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx +++ b/roofit/roofitcore/src/RooAbsMinimizerFcn.cxx @@ -68,8 +68,7 @@ RooAbsMinimizerFcn::RooAbsMinimizerFcn(RooArgList paramList, RooMinimizer *conte /// Internal function to synchronize TMinimizer with current /// information in RooAbsReal function parameters -bool RooAbsMinimizerFcn::synchronizeParameterSettings(std::vector ¶meters, - bool optConst) +bool RooAbsMinimizerFcn::synchronizeParameterSettings(std::vector ¶meters) { // Synchronize MINUIT with function state @@ -144,24 +143,12 @@ bool RooAbsMinimizerFcn::synchronizeParameterSettings(std::vector ¶meters) { - return synchronizeParameterSettings(parameters, _optConst); + return synchronizeParameterSettings(parameters); } /// Transfer MINUIT fit results back into RooFit objects. @@ -304,55 +291,6 @@ void RooAbsMinimizerFcn::finishDoEval() const _evalCounter++; } -void RooAbsMinimizerFcn::setOptimizeConst(int flag) -{ - auto ctx = _context->makeEvalErrorContext(); - - if (_optConst && !flag) { - if (_context->getPrintLevel() > -1) { - oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: deactivating const optimization" - << std::endl; - } - setOptimizeConstOnFunction(RooAbsArg::DeActivate, true); - _optConst = flag; - } else if (!_optConst && flag) { - if (_context->getPrintLevel() > -1) { - oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: activating const optimization" - << std::endl; - } - setOptimizeConstOnFunction(RooAbsArg::Activate, flag > 1); - _optConst = flag; - } else if (_optConst && flag) { - if (_context->getPrintLevel() > -1) { - oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: const optimization already active" - << std::endl; - } - } else { - if (_context->getPrintLevel() > -1) { - oocoutI(_context, Minimization) << "RooAbsMinimizerFcn::setOptimizeConst: const optimization wasn't active" - << std::endl; - } - } -} - -void RooAbsMinimizerFcn::optimizeConstantTerms(bool constStatChange, bool constValChange) -{ - auto ctx = _context->makeEvalErrorContext(); - - if (constStatChange) { - - oocoutI(_context, Minimization) - << "RooAbsMinimizerFcn::optimizeConstantTerms: set of constant parameters changed, rerunning const optimizer" - << std::endl; - setOptimizeConstOnFunction(RooAbsArg::ConfigChange, true); - } else if (constValChange) { - oocoutI(_context, Minimization) - << "RooAbsMinimizerFcn::optimizeConstantTerms: constant parameter values changed, rerunning const optimizer" - << std::endl; - setOptimizeConstOnFunction(RooAbsArg::ValueChange, true); - } -} - RooArgList RooAbsMinimizerFcn::floatParams() const { RooArgList out; diff --git a/roofit/roofitcore/src/RooAbsMinimizerFcn.h b/roofit/roofitcore/src/RooAbsMinimizerFcn.h index 68796e26e9a9b..2607f9e0bc20b 100644 --- a/roofit/roofitcore/src/RooAbsMinimizerFcn.h +++ b/roofit/roofitcore/src/RooAbsMinimizerFcn.h @@ -42,7 +42,7 @@ class RooAbsMinimizerFcn { virtual void initMinimizer(ROOT::Math::Minimizer &, RooMinimizer *context) = 0; /// Informs Minuit through its parameter_settings vector of RooFit parameter properties. - bool synchronizeParameterSettings(std::vector ¶meters, bool optConst); + bool synchronizeParameterSettings(std::vector ¶meters); /// Like synchronizeParameterSettings, Synchronize informs Minuit through /// its parameter_settings vector of RooFit parameter properties, but /// Synchronize can be overridden to e.g. also include gradient strategy @@ -77,8 +77,6 @@ class RooAbsMinimizerFcn { unsigned int getNDim() const { return _floatableParamIndices.size(); } - void setOptimizeConst(Int_t flag); - bool SetPdfParamVal(int index, double value) const; /// Enable or disable offsetting on the function to be minimized, which enhances numerical precision. @@ -94,11 +92,6 @@ class RooAbsMinimizerFcn { virtual RooArgSet freezeDisconnectedParameters() const { return {}; } protected: - void optimizeConstantTerms(bool constStatChange, bool constValChange); - /// This function must be overridden in the derived class to pass on constant term optimization configuration - /// to the function to be minimized. For a RooAbsArg, this would be RooAbsArg::constOptimizeTestStatistic. - virtual void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) = 0; - void printEvalErrors() const; double applyEvalErrorHandling(double fvalue) const; @@ -123,8 +116,6 @@ class RooAbsMinimizerFcn { // functions to be actually const (even though state still changes in the // error handling object). - bool _optConst = false; - RooArgList _allParams; RooArgList _allParamsInit; diff --git a/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx b/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx index b9e32cff6337f..e39e9aec3c05e 100644 --- a/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx +++ b/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx @@ -410,91 +410,6 @@ void RooAbsOptTestStatistic::printCompactTreeHook(ostream& os, const char* inden -//////////////////////////////////////////////////////////////////////////////// -/// Driver function to propagate constant term optimizations in test statistic. -/// If code Activate is sent, constant term optimization will be executed. -/// If code Deactivate is sent, any existing constant term optimizations will -/// be abandoned. If codes ConfigChange or ValueChange are sent, any existing -/// constant term optimizations will be redone. - -void RooAbsOptTestStatistic::constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - static bool hasWarned = false; - if (!hasWarned) { - std::stringstream ss; - ss << "Deprecated constant term optimization detected,\n" - << "enabled via RooFit::Optimize() or RooMinimizer::optimizeConst():\n" - << " This functionality only affects the legacy evaluation backend.\n" - << " The vectorized CPU backend performs const term optimization automatically.\n" - << " The option is ignored and will be removed in ROOT 6.42.\n" - << " Should your fit not be possible without the legacy backend, please open a GitHub issue.\n"; - oocoutW(static_cast(nullptr), InputArguments) << ss.str() << std::endl; - hasWarned = true; - } - - // std::cout << "ROATS::constOpt(" << GetName() << ") funcClone structure dump BEFORE const-opt" << std::endl ; - // _funcClone->Print("t") ; - - RooAbsTestStatistic::constOptimizeTestStatistic(opcode,doAlsoTrackingOpt); - if (operMode()!=Slave) return ; - - if (_dataClone->hasFilledCache() && _dataClone->store()->cacheOwner()!=this) { - if (opcode==Activate) { - cxcoutW(Optimization) << "RooAbsOptTestStatistic::constOptimize(" << GetName() - << ") dataset cache is owned by another object, no constant term optimization can be applied" << std::endl ; - } - return ; - } - - if (!allowFunctionCache()) { - if (opcode==Activate) { - cxcoutI(Optimization) << "RooAbsOptTestStatistic::constOptimize(" << GetName() - << ") function caching prohibited by test statistic, no constant term optimization is applied" << std::endl ; - } - return ; - } - - if (_dataClone->hasFilledCache() && opcode==Activate) { - opcode=ValueChange ; - } - - switch(opcode) { - case Activate: - cxcoutI(Optimization) << "RooAbsOptTestStatistic::constOptimize(" << GetName() - << ") optimizing evaluation of test statistic by finding all nodes in p.d.f that depend exclusively" - << " on observables and constant parameters and precalculating their values" << std::endl ; - optimizeConstantTerms(true,doAlsoTrackingOpt) ; - break ; - - case DeActivate: - cxcoutI(Optimization) << "RooAbsOptTestStatistic::constOptimize(" << GetName() - << ") deactivating optimization of constant terms in test statistic" << std::endl ; - optimizeConstantTerms(false) ; - break ; - - case ConfigChange: - cxcoutI(Optimization) << "RooAbsOptTestStatistic::constOptimize(" << GetName() - << ") one ore more parameter were changed from constant to floating or vice versa, " - << "re-evaluating constant term optimization" << std::endl ; - optimizeConstantTerms(false) ; - optimizeConstantTerms(true,doAlsoTrackingOpt) ; - break ; - - case ValueChange: - cxcoutI(Optimization) << "RooAbsOptTestStatistic::constOptimize(" << GetName() - << ") the value of one ore more constant parameter were changed re-evaluating constant term optimization" << std::endl ; - // Request a forcible cache update of all cached nodes - _dataClone->store()->forceCacheUpdate() ; - - break ; - } - -// std::cout << "ROATS::constOpt(" << GetName() << ") funcClone structure dump AFTER const-opt" << std::endl ; -// _funcClone->Print("t") ; -} - - - //////////////////////////////////////////////////////////////////////////////// /// This method changes the value caching logic for all nodes that depends on any of the observables /// as defined by the given dataset. When evaluating a test statistic constructed from the RooAbsReal @@ -517,133 +432,6 @@ void RooAbsOptTestStatistic::optimizeCaching() // Disable propagation of dirty state flags for observables _dataClone->setDirtyProp(false) ; - - // Disable reading of observables that are not used - _dataClone->optimizeReadingWithCaching(*_funcClone, RooArgSet(),requiredExtraObservables()) ; -} - - - -//////////////////////////////////////////////////////////////////////////////// -/// Driver function to activate global constant term optimization. -/// If activated, constant terms are found and cached with the dataset. -/// The operation mode of cached nodes is set to AClean meaning that -/// their getVal() call will never result in an evaluate call. -/// Finally the branches in the dataset that correspond to observables -/// that are exclusively used in constant terms are disabled as -/// they serve no more purpose - -void RooAbsOptTestStatistic::optimizeConstantTerms(bool activate, bool applyTrackingOpt) -{ - if(activate) { - - if (_optimized) { - return ; - } - - // Trigger create of all object caches now in nodes that have deferred object creation - // so that cache contents can be processed immediately - _funcClone->getVal(_normSet) ; - - - // WVE - Patch to allow customization of optimization level per component pdf - if (_funcClone->getAttribute("NoOptimizeLevel1")) { - coutI(Minimization) << " Optimization customization: Level-1 constant-term optimization prohibited by attribute NoOptimizeLevel1 set on top-level pdf " - << _funcClone->ClassName() << "::" << _funcClone->GetName() << std::endl ; - return ; - } - if (_funcClone->getAttribute("NoOptimizeLevel2")) { - coutI(Minimization) << " Optimization customization: Level-2 constant-term optimization prohibited by attribute NoOptimizeLevel2 set on top-level pdf " - << _funcClone->ClassName() << "::" << _funcClone->GetName() << std::endl ; - applyTrackingOpt=false ; - } - - // Apply tracking optimization here. Default strategy is to track components - // of RooAddPdfs and RooRealSumPdfs. If these components are a RooProdPdf - // or a RooProduct respectively, track the components of these products instead - // of the product term - RooArgSet trackNodes ; - - - // Add safety check here - applyTrackingOpt will only be applied if present - // dataset is constructed in terms of a RooVectorDataStore - if (applyTrackingOpt) { - if (!dynamic_cast(_dataClone->store())) { - coutW(Optimization) << "RooAbsOptTestStatistic::optimizeConstantTerms(" << GetName() - << ") WARNING Cache-and-track optimization (Optimize level 2) is only available for datasets" - << " implement in terms of RooVectorDataStore - ignoring this option for current dataset" << std::endl ; - applyTrackingOpt = false ; - } - } - - if (applyTrackingOpt) { - RooArgSet branches ; - _funcClone->branchNodeServerList(&branches) ; - for (auto arg : branches) { - arg->setCacheAndTrackHints(trackNodes); - } - // Do not set CacheAndTrack on constant expressions - trackNodes.remove(*std::unique_ptr{trackNodes.selectByAttrib("Constant",true)}); - - // Set CacheAndTrack flag on all remaining nodes - trackNodes.setAttribAll("CacheAndTrack",true) ; - } - - // Find all nodes that depend exclusively on constant parameters - _cachedNodes.removeAll() ; - - _funcClone->findConstantNodes(*_dataClone->get(),_cachedNodes) ; - - // Cache constant nodes with dataset - also cache entries corresponding to zero-weights in data when using BinnedLikelihood - _dataClone->cacheArgs(this,_cachedNodes,_normSet, _skipZeroWeights); - - // Put all cached nodes in AClean value caching mode so that their evaluate() is never called - for (auto cacheArg : _cachedNodes) { - cacheArg->setOperMode(RooAbsArg::AClean) ; - } - - std::unique_ptr constNodes{_cachedNodes.selectByAttrib("ConstantExpressionCached",true)}; - RooArgSet actualTrackNodes(_cachedNodes) ; - actualTrackNodes.remove(*constNodes) ; - if (!constNodes->empty()) { - if (constNodes->size()<20) { - coutI(Minimization) << " The following expressions have been identified as constant and will be precalculated and cached: " << *constNodes << std::endl ; - } else { - coutI(Minimization) << " A total of " << constNodes->size() << " expressions have been identified as constant and will be precalculated and cached." << std::endl ; - } - } - if (!actualTrackNodes.empty()) { - if (actualTrackNodes.size()<20) { - coutI(Minimization) << " The following expressions will be evaluated in cache-and-track mode: " << actualTrackNodes << std::endl ; - } else { - coutI(Minimization) << " A total of " << constNodes->size() << " expressions will be evaluated in cache-and-track-mode." << std::endl ; - } - } - - // Disable reading of observables that are no longer used - _dataClone->optimizeReadingWithCaching(*_funcClone, _cachedNodes,requiredExtraObservables()) ; - - _optimized = true ; - - } else { - - // Delete the cache - _dataClone->resetCache() ; - - // Reactivate all tree branches - _dataClone->setArgStatus(*_dataClone->get(),true) ; - - // Reset all nodes to ADirty - optimizeCaching() ; - - // Disable propagation of dirty state flags for observables - _dataClone->setDirtyProp(false) ; - - _cachedNodes.removeAll() ; - - - _optimized = false ; - } } @@ -700,11 +488,6 @@ bool RooAbsOptTestStatistic::setDataSlave(RooAbsData& indata, bool cloneData, bo _dataClone->setDirtyProp(false) ; _data = _dataClone ; - // ReCache constant nodes with dataset - if (!_cachedNodes.empty()) { - _dataClone->cacheArgs(this,_cachedNodes,_normSet, _skipZeroWeights); - } - // Adjust internal event count setEventCount(indata.numEntries()) ; @@ -779,10 +562,4 @@ const char* RooAbsOptTestStatistic::cacheUniqueSuffix() const { return Form("_%lx", _dataClone->uniqueId().value()) ; } - -void RooAbsOptTestStatistic::runRecalculateCache(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const -{ - _dataClone->store()->recalculateCache(_projDeps, firstEvent, lastEvent, stepSize, _skipZeroWeights); -} - /// \endcond diff --git a/roofit/roofitcore/src/RooAbsOptTestStatistic.h b/roofit/roofitcore/src/RooAbsOptTestStatistic.h index 23a9a2e7766f8..17ba39d2265d0 100644 --- a/roofit/roofitcore/src/RooAbsOptTestStatistic.h +++ b/roofit/roofitcore/src/RooAbsOptTestStatistic.h @@ -63,14 +63,11 @@ class RooAbsOptTestStatistic : public RooAbsTestStatistic { friend class RooAbsTestStatistic ; virtual bool allowFunctionCache() { return true ; } - void constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt=true) override ; bool redirectServersHook(const RooAbsCollection& newServerList, bool mustReplaceAll, bool nameChange, bool isRecursive) override ; void printCompactTreeHook(std::ostream& os, const char* indent="") override ; virtual RooArgSet requiredExtraObservables() const { return RooArgSet() ; } void optimizeCaching() ; - void optimizeConstantTerms(bool,bool=true) ; - void runRecalculateCache(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const override; RooArgSet* _normSet = nullptr; ///< Pointer to set with observables used for normalization RooArgSet* _funcCloneSet = nullptr; ///< Set owning all components of internal clone of input function diff --git a/roofit/roofitcore/src/RooAbsPdf.cxx b/roofit/roofitcore/src/RooAbsPdf.cxx index 7aad2f55d3b9f..aea212e2d3e60 100644 --- a/roofit/roofitcore/src/RooAbsPdf.cxx +++ b/roofit/roofitcore/src/RooAbsPdf.cxx @@ -867,9 +867,6 @@ double RooAbsPdf::extendedTerm(RooAbsData const& data, bool weightSquared, bool * **codegen_no_grad** **Experimental** - Same as **codegen**, but doesn't generate and compile the gradient code and use the regular numerical differentiation instead. * This is expected to be slower, but useful for debugging problems with the analytic gradient. * - * `Optimize(bool flag)` Activate constant term optimization. - * Only relevant for `legacy` evaluation backend and off by default, as the default `cpu` backend already includes this optimization unconditionally. - * \warning Deprecated option that will be removed in ROOT 6.42! * `SplitRange(bool flag)` Use separate fit ranges in a simultaneous fit. Actual range name for each subsample is assumed to * be `rangeName_indexState`, where `indexState` is the state of the master index category of the simultaneous fit. * Using `Range("range"), SplitRange()` as switches, different ranges could be set like this: @@ -980,9 +977,6 @@ std::unique_ptr RooAbsPdf::createNLLImpl(RooAbsData &data, const Roo * * * `InitialHesse(bool flag)` Flag controls if HESSE before MIGRAD as well, off by default - * `Optimize(bool flag)` Activate constant term optimization of test statistic during minimization. - * Only relevant for `legacy` evaluation backend and off by default, as the default `cpu` backend already includes this optimization unconditionally. - * \warning Deprecated option that will be removed in ROOT 6.42! * `Hesse(bool flag)` Flag controls if HESSE is run after MIGRAD, on by default * `Minos(bool flag)` Flag controls if MINOS is run after HESSE, off by default * `Minos(const RooArgSet& set)` Only run MINOS on given subset of arguments diff --git a/roofit/roofitcore/src/RooAbsReal.cxx b/roofit/roofitcore/src/RooAbsReal.cxx index b6e5a1f901749..cfd92030daa08 100644 --- a/roofit/roofitcore/src/RooAbsReal.cxx +++ b/roofit/roofitcore/src/RooAbsReal.cxx @@ -3979,9 +3979,6 @@ double RooAbsReal::findRoot(RooRealVar& x, double xmin, double xmax, double yval /// `Range(double lo, double hi)` Fit only data inside given range. A range named "fit" is created on the fly on all observables. /// Multiple comma separated range names can be specified. /// `NumCPU(int num)` Parallelize NLL calculation on num CPUs -/// `Optimize(bool flag)` Activate constant term optimization. -/// Only relevant for `legacy` evaluation backend and off by default, as the default `cpu` backend already includes this optimization unconditionally. -/// \warning Deprecated option that will be removed in ROOT 6.42! /// `IntegrateBins()` Integrate PDF within each bin. This sets the desired precision. /// `Verbose()` Verbose output of GOF framework /// `SumCoefRange()` Set the range in which to interpret the coefficients of RooAddPdf components diff --git a/roofit/roofitcore/src/RooAbsTestStatistic.cxx b/roofit/roofitcore/src/RooAbsTestStatistic.cxx index b2dbe20e6f89d..993d18a2f306f 100644 --- a/roofit/roofitcore/src/RooAbsTestStatistic.cxx +++ b/roofit/roofitcore/src/RooAbsTestStatistic.cxx @@ -258,7 +258,6 @@ double RooAbsTestStatistic::evaluate() const break ; } - runRecalculateCache(nFirst, nLast, nStep); double ret = evaluatePartition(nFirst,nLast,nStep); if (numSets()==1) { @@ -340,33 +339,6 @@ void RooAbsTestStatistic::printCompactTreeHook(ostream& os, const char* indent) -//////////////////////////////////////////////////////////////////////////////// -/// Forward constant term optimization management calls to component -/// test statistics - -void RooAbsTestStatistic::constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - initialize(); - if (SimMaster == _gofOpMode) { - // Forward to slaves - int i = 0; - for (auto& gof : _gofArray) { - // In SimComponents Splitting strategy only constOptimize the terms that are actually used - RooFit::MPSplit effSplit = (_mpinterl!=RooFit::Hybrid) ? _mpinterl : gof->_mpinterl; - if ( (i % _numSets == _setNum) || (effSplit != RooFit::SimComponents) ) { - gof->constOptimizeTestStatistic(opcode,doAlsoTrackingOpt); - } - ++i; - } - } else if (MPMaster == _gofOpMode) { - for (Int_t i = 0; i < _nCPU; ++i) { - _mpfeArray[i]->constOptimizeTestStatistic(opcode,doAlsoTrackingOpt); - } - } -} - - - //////////////////////////////////////////////////////////////////////////////// /// Set MultiProcessor set number identification of this instance diff --git a/roofit/roofitcore/src/RooAbsTestStatistic.h b/roofit/roofitcore/src/RooAbsTestStatistic.h index 3dbf2d7811ee5..4e63cbfe519e1 100644 --- a/roofit/roofitcore/src/RooAbsTestStatistic.h +++ b/roofit/roofitcore/src/RooAbsTestStatistic.h @@ -57,8 +57,6 @@ class RooAbsTestStatistic : public RooAbsReal { virtual RooAbsTestStatistic* create(const char *name, const char *title, RooAbsReal& real, RooAbsData& data, const RooArgSet& projDeps, Configuration const& cfg) = 0; - void constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt=true) override ; - virtual double combinedValue(RooAbsReal** gofArray, Int_t nVal) const = 0 ; virtual double globalNormalization() const { // Default value of global normalization factor is 1.0 @@ -88,9 +86,6 @@ class RooAbsTestStatistic : public RooAbsReal { virtual double evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const = 0 ; virtual double getCarry() const; - // Overridden in cache-optimized test statistic - virtual void runRecalculateCache(std::size_t /*firstEvent*/, std::size_t /*lastEvent*/, std::size_t /*stepSize*/) const {} - void setMPSet(Int_t setNum, Int_t numSets) ; void setSimCount(Int_t simCount) { // Store total number of components p.d.f. of a RooSimultaneous in this component test statistic diff --git a/roofit/roofitcore/src/RooCompositeDataStore.cxx b/roofit/roofitcore/src/RooCompositeDataStore.cxx index 19f26741961e0..a838a12c25f17 100644 --- a/roofit/roofitcore/src/RooCompositeDataStore.cxx +++ b/roofit/roofitcore/src/RooCompositeDataStore.cxx @@ -160,40 +160,6 @@ std::unique_ptr RooCompositeDataStore::reduce( } -//////////////////////////////////////////////////////////////////////////////// -/// Forward recalculate request to all subsets - -void RooCompositeDataStore::recalculateCache(const RooArgSet* proj, Int_t firstEvent, Int_t lastEvent, Int_t stepSize, bool skipZeroWeights) -{ - for (auto const& item : _dataMap) { - item.second->recalculateCache(proj,firstEvent,lastEvent,stepSize,skipZeroWeights) ; - } -} - - -//////////////////////////////////////////////////////////////////////////////// - -bool RooCompositeDataStore::hasFilledCache() const -{ - bool ret(false) ; - for (auto const& item : _dataMap) { - ret |= item.second->hasFilledCache() ; - } - return ret ; -} - - -//////////////////////////////////////////////////////////////////////////////// - -void RooCompositeDataStore::forceCacheUpdate() -{ - for (auto const& item : _dataMap) { - item.second->forceCacheUpdate() ; - } -} - - - //////////////////////////////////////////////////////////////////////////////// /// Forward fill request to appropriate subset @@ -396,57 +362,6 @@ void RooCompositeDataStore::reset() -//////////////////////////////////////////////////////////////////////////////// - -void RooCompositeDataStore::cacheArgs(const RooAbsArg* owner, RooArgSet& newVarSet, const RooArgSet* nset, bool skipZeroWeights) -{ - for (auto const& item : _dataMap) { - item.second->cacheArgs(owner,newVarSet,nset,skipZeroWeights) ; - } -} - - - -//////////////////////////////////////////////////////////////////////////////// - -void RooCompositeDataStore::setArgStatus(const RooArgSet& set, bool active) -{ - for (auto const& item : _dataMap) { - RooArgSet subset; - set.selectCommon(*item.second->get(), subset); - item.second->setArgStatus(subset,active) ; - } - return ; -} - - - -//////////////////////////////////////////////////////////////////////////////// -/// Initialize cache of dataset: attach variables of cache ArgSet -/// to the corresponding TTree branches - -void RooCompositeDataStore::attachCache(const RooAbsArg* newOwner, const RooArgSet& inCachedVars) -{ - for (auto const& item : _dataMap) { - item.second->attachCache(newOwner,inCachedVars) ; - } - return ; -} - - - -//////////////////////////////////////////////////////////////////////////////// - -void RooCompositeDataStore::resetCache() -{ - for (auto const& item : _dataMap) { - item.second->resetCache() ; - } - return ; -} - - - //////////////////////////////////////////////////////////////////////////////// void RooCompositeDataStore::attachBuffers(const RooArgSet& extObs) diff --git a/roofit/roofitcore/src/RooDataSet.cxx b/roofit/roofitcore/src/RooDataSet.cxx index fcc1bae954c13..7828d6715a334 100644 --- a/roofit/roofitcore/src/RooDataSet.cxx +++ b/roofit/roofitcore/src/RooDataSet.cxx @@ -624,7 +624,6 @@ std::unique_ptr RooDataSet::reduceEng(const RooArgSet &varSubset, co if (!cutRange || strchr(cutRange, ',') == nullptr) { auto &ds = static_cast(*out); ds._dstore = _dstore->reduce(ds.GetName(), ds.GetTitle(), ds._vars, cutVar, cutRange, nStart, nStop); - ds._cachedVars.add(_dstore->cachedVars()); } else { // Composite case: multiple ranges auto tokens = ROOT::Split(cutRange, ","); @@ -637,7 +636,6 @@ std::unique_ptr RooDataSet::reduceEng(const RooArgSet &varSubset, co std::unique_ptr appendedData{createEmptyClone()}; auto &ds = static_cast(*appendedData); ds._dstore = _dstore->reduce(ds.GetName(), ds.GetTitle(), ds._vars, cutVar, token.c_str(), nStart, nStop); - ds._cachedVars.add(_dstore->cachedVars()); static_cast(*out).append(ds); } } diff --git a/roofit/roofitcore/src/RooGlobalFunc.cxx b/roofit/roofitcore/src/RooGlobalFunc.cxx index bc0422d3b7d19..5fd0beb2f8115 100644 --- a/roofit/roofitcore/src/RooGlobalFunc.cxx +++ b/roofit/roofitcore/src/RooGlobalFunc.cxx @@ -636,10 +636,6 @@ RooCmdArg PrefitDataFraction(double data_ratio) { return RooCmdArg("Prefit", 0, 0, data_ratio, 0, nullptr, nullptr, nullptr, nullptr); } -RooCmdArg Optimize(Int_t flag) -{ - return RooCmdArg("Optimize", flag); -} RooCmdArg Verbose(bool flag) { return RooCmdArg("Verbose", flag); diff --git a/roofit/roofitcore/src/RooMinimizer.cxx b/roofit/roofitcore/src/RooMinimizer.cxx index 718a13880ac2b..fd2900fe69812 100644 --- a/roofit/roofitcore/src/RooMinimizer.cxx +++ b/roofit/roofitcore/src/RooMinimizer.cxx @@ -33,8 +33,7 @@ in the constant status etc is forwarded to MINUIT prior to execution of the MINUIT call. Afterwards the RooFit objects are resynchronized with the output state of MINUIT: changes parameter values, errors are propagated. -Various methods are available to control verbosity, profiling, -automatic PDF optimization. +Various methods are available to control verbosity or profiling. **/ #include "RooMinimizer.h" @@ -538,18 +537,6 @@ int RooMinimizer::getPrintLevel() return _config.MinimizerOptions().PrintLevel() + 1; } -//////////////////////////////////////////////////////////////////////////////// -/// If flag is true, perform constant term optimization on -/// function being minimized. -/// -/// \deprecated Will be removed in ROOT 6.42, as this functionality only affects the legacy evalution backend. -/// The default `cpu` backend already includes this optimization unconditionally. - -void RooMinimizer::optimizeConst(int flag) -{ - _fcn->setOptimizeConst(flag); -} - //////////////////////////////////////////////////////////////////////////////// /// Save and return a RooFitResult snapshot of current minimizer status. /// This snapshot contains the values of all constant parameters, diff --git a/roofit/roofitcore/src/RooMinimizerFcn.cxx b/roofit/roofitcore/src/RooMinimizerFcn.cxx index 469491ee6bcbc..71fedcf48bcf8 100644 --- a/roofit/roofitcore/src/RooMinimizerFcn.cxx +++ b/roofit/roofitcore/src/RooMinimizerFcn.cxx @@ -73,11 +73,6 @@ RooMinimizerFcn::RooMinimizerFcn(RooAbsReal *funct, RooMinimizer *context) } } -void RooMinimizerFcn::setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - _funct->constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); -} - /// Evaluate function given the parameters in `x`. double RooMinimizerFcn::operator()(const double *x) const { diff --git a/roofit/roofitcore/src/RooMinimizerFcn.h b/roofit/roofitcore/src/RooMinimizerFcn.h index 9189b6bba81e6..48c3cad015046 100644 --- a/roofit/roofitcore/src/RooMinimizerFcn.h +++ b/roofit/roofitcore/src/RooMinimizerFcn.h @@ -40,8 +40,6 @@ class RooMinimizerFcn : public RooAbsMinimizerFcn { std::string getFunctionName() const override; std::string getFunctionTitle() const override; - void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override; - void setOffsetting(bool flag) override; double operator()(const double *x) const; diff --git a/roofit/roofitcore/src/RooRealIntegral.cxx b/roofit/roofitcore/src/RooRealIntegral.cxx index c580bb0b2c10e..74b9a4234afa2 100644 --- a/roofit/roofitcore/src/RooRealIntegral.cxx +++ b/roofit/roofitcore/src/RooRealIntegral.cxx @@ -822,32 +822,12 @@ double RooRealIntegral::evaluate() const return 0; } - // Find any function dependents that are "AClean" and switch them temporarily to "Auto". - // We do this by compute graph traversal and RAII objects on the heap, - // which seems quite expensive, but is not as bad as it looks because: - // 1. The sub-graphs representing numerically-integrated functions - // are usually small - // 2. The numerical integration itself dominates the runtime of the - // evaluation. - // 3. The operMode is only "AClean" if we use the constant term - // optimization of the legacy test statistics. - // 4. Once the legacy test statistics are deprecated and removed, - // this code block can go away (TODO when that happens). - // Note: in the past, the "AClean" states were changed with a global - // setDirtyInhibit(true) before evaluating the numeric integral. While - // this avoids the bookkeeping overhead, it actually changes the oper - // mode of all nodes to "ADirty" and not to "Auto", resulting in - // significant performance loss in case the target function benefits - // from caching subgraph results (e.g. for nested numeric integrals). + // Not clear why this is still needed RooArgList serverList; _function->treeNodeServerList(&serverList, nullptr, true, true, false, true); std::list operModeRAII; - for (auto *arg : serverList) { arg->syncCache(); - if (arg->operMode() == RooAbsArg::AClean) { - operModeRAII.emplace_back(arg, RooAbsArg::Auto); - } } // Save current integral dependent values @@ -857,9 +837,6 @@ double RooRealIntegral::evaluate() const // Evaluate sum/integral retVal = sum() ; - // This must happen BEFORE restoring dependents, otherwise no dirty state propagation in restore step - operModeRAII.clear(); - // Restore integral dependent values _intList.assign(_saveInt) ; _sumList.assign(_saveSum) ; diff --git a/roofit/roofitcore/src/RooRealMPFE.cxx b/roofit/roofitcore/src/RooRealMPFE.cxx index cd87846508e01..975d2d25289b6 100644 --- a/roofit/roofitcore/src/RooRealMPFE.cxx +++ b/roofit/roofitcore/src/RooRealMPFE.cxx @@ -348,17 +348,6 @@ void RooRealMPFE::serverLoop() } break; - case ConstOpt: - { - bool doTrack ; - int code; - *_pipe >> code >> doTrack; - if (_verboseServer) std::cout << "RooRealMPFE::serverLoop(" << GetName() - << ") IPC fromClient> ConstOpt " << code << " doTrack = " << (doTrack?"T":"F") << std::endl ; - ((RooAbsReal&)_arg.arg()).constOptimizeTestStatistic(static_cast(code),doTrack) ; - break ; - } - case Verbose: { bool flag ; @@ -675,33 +664,6 @@ void RooRealMPFE::standby() } - -//////////////////////////////////////////////////////////////////////////////// -/// Intercept call to optimize constant term in test statistics -/// and forward it to object on server side. - -void RooRealMPFE::constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTracking) -{ -#ifndef _WIN32 - if (_state==Client) { - - int msg = ConstOpt ; - int op = opcode; - *_pipe << msg << op << doAlsoTracking; - if (_verboseServer) std::cout << "RooRealMPFE::constOptimize(" << GetName() - << ") IPC toServer> ConstOpt " << opcode << std::endl ; - - initVars() ; - } -#endif // _WIN32 - - if (_state==Inline) { - ((RooAbsReal&)_arg.arg()).constOptimizeTestStatistic(opcode,doAlsoTracking) ; - } -} - - - //////////////////////////////////////////////////////////////////////////////// /// Control verbose messaging related to inter process communication /// on both client and server side diff --git a/roofit/roofitcore/src/RooRealMPFE.h b/roofit/roofitcore/src/RooRealMPFE.h index 4174cacda3723..31a0b60d9eafc 100644 --- a/roofit/roofitcore/src/RooRealMPFE.h +++ b/roofit/roofitcore/src/RooRealMPFE.h @@ -55,7 +55,6 @@ class RooRealMPFE : public RooAbsReal { // Function evaluation double evaluate() const override ; friend class RooAbsTestStatistic ; - void constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTracking=true) override ; virtual double getCarry() const; enum State { Initialize,Client,Server,Inline } ; diff --git a/roofit/roofitcore/src/RooTreeDataStore.cxx b/roofit/roofitcore/src/RooTreeDataStore.cxx index 79e4e6ca5f31b..8fcf975b9199b 100644 --- a/roofit/roofitcore/src/RooTreeDataStore.cxx +++ b/roofit/roofitcore/src/RooTreeDataStore.cxx @@ -165,12 +165,6 @@ RooTreeDataStore::RooTreeDataStore(RooStringView name, RooStringView title, RooA // Constructor from existing data set with list of variables that preserves the cache initialize(); - attachCache(nullptr,(static_cast(tds))._cachedVars) ; - - // WVE copy values of cached variables here!!! - _cacheTree->CopyEntries((static_cast(tds))._cacheTree) ; - _cacheOwner = nullptr ; - loadValues(&tds,cloneVar.get(),cutRange,nStart,nStop); } @@ -219,29 +213,6 @@ RooRealVar* RooTreeDataStore::weightVar(const RooArgSet& allVars, const char* wg } - - -//////////////////////////////////////////////////////////////////////////////// -/// Initialize cache of dataset: attach variables of cache ArgSet -/// to the corresponding TTree branches - -void RooTreeDataStore::attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVarsIn) -{ - // iterate over the cache variables for this dataset - _cachedVars.removeAll() ; - for (RooAbsArg * var : cachedVarsIn) { - var->attachToTree(*_cacheTree,_defTreeBufSize) ; - _cachedVars.add(*var) ; - } - _cacheOwner = newOwner ; - -} - - - - - - //////////////////////////////////////////////////////////////////////////////// RooTreeDataStore::RooTreeDataStore(const RooTreeDataStore& other, const char* newname) : @@ -292,9 +263,6 @@ RooTreeDataStore::~RooTreeDataStore() if (_tree) { delete _tree ; } - if (_cacheTree) { - delete _cacheTree ; - } } @@ -342,12 +310,6 @@ void RooTreeDataStore::createTree(RooStringView name, RooStringView title) gDirectory->cd(memDir) ; } - if (!_cacheTree) { - _cacheTree = new TTree(TString{name.c_str()} + "_cacheTree", TString{title.c_str()}); - _cacheTree->SetDirectory(nullptr) ; - gDirectory->RecursiveRemove(_cacheTree) ; - } - if (notInMemNow) { gDirectory->cd(pwd) ; } @@ -522,7 +484,6 @@ void RooTreeDataStore::loadValues(const RooAbsDataStore *ads, const RooFormulaVa continue ; } - _cachedVars.assign(static_cast(ads)->_cachedVars) ; fill() ; } @@ -554,7 +515,6 @@ const RooArgSet* RooTreeDataStore::get(Int_t index) const checkInit() ; Int_t ret = _tree->GetEntry(index, 1); - _cacheTree->GetEntry(index,1); if(!ret) return nullptr; @@ -563,11 +523,6 @@ const RooArgSet* RooTreeDataStore::get(Int_t index) const for (auto var : _vars) { var->setValueDirty(); // This triggers recalculation of all clients } - - for (auto var : _cachedVars) { - var->setValueDirty(); // This triggers recalculation of all clients, but doesn't recalculate self - var->clearValueDirty(); - } } // Update current weight cache @@ -939,98 +894,6 @@ void RooTreeDataStore::reset() -//////////////////////////////////////////////////////////////////////////////// -/// Cache given RooAbsArgs with this tree: The tree is -/// given direct write access of the args internal cache -/// the args values is pre-calculated for all data points -/// in this data collection. Upon a get() call, the -/// internal cache of 'newVar' will be loaded with the -/// precalculated value and it's dirty flag will be cleared. - -void RooTreeDataStore::cacheArgs(const RooAbsArg* owner, RooArgSet& newVarSet, const RooArgSet* nset, bool /*skipZeroWeights*/) -{ - checkInit() ; - - _cacheOwner = owner ; - - std::unique_ptr constExprVarSet{newVarSet.selectByAttrib("ConstantExpression", true)}; - - bool doTreeFill = (_cachedVars.empty()) ; - - for (RooAbsArg * arg : *constExprVarSet) { - // Attach original newVar to this tree - arg->attachToTree(*_cacheTree,_defTreeBufSize) ; - //arg->recursiveRedirectServers(_vars) ; - _cachedVars.add(*arg) ; - } - - // WVE need to reset TTRee buffers to original datamembers here - //resetBuffers() ; - - // Refill regular and cached variables of current tree from clone - for (int i=0 ; i < _tree->GetEntries() ; i++) { - get(i) ; - - // Evaluate the cached variables and store the results - for (RooAbsArg * arg : *constExprVarSet) { - arg->setValueDirty() ; - arg->syncCache(nset) ; - if (!doTreeFill) { - arg->fillTreeBranch(*_cacheTree) ; - } - } - - if (doTreeFill) { - _cacheTree->Fill() ; - } - } - - // WVE need to restore TTRee buffers to previous values here - //restoreAlternateBuffers() ; -} - - - - -//////////////////////////////////////////////////////////////////////////////// -/// Activate or deactivate the branch status of the TTree branch associated -/// with the given set of dataset observables - -void RooTreeDataStore::setArgStatus(const RooArgSet& set, bool active) -{ - for (RooAbsArg * arg : set) { - RooAbsArg* depArg = _vars.find(arg->GetName()) ; - if (!depArg) { - coutE(InputArguments) << "RooTreeDataStore::setArgStatus(" << GetName() - << ") dataset doesn't contain variable " << arg->GetName() << std::endl ; - continue ; - } - depArg->setTreeBranchStatus(*_tree,active) ; - } -} - - - -//////////////////////////////////////////////////////////////////////////////// -/// Remove tree with values of cached observables -/// and clear list of cached observables - -void RooTreeDataStore::resetCache() -{ - // Empty list of cached functions - _cachedVars.removeAll() ; - - // Delete & recreate cache tree - delete _cacheTree ; - _cacheTree = nullptr ; - createTree(makeTreeName().c_str(), GetTitle()); - - return ; -} - - - - //////////////////////////////////////////////////////////////////////////////// void RooTreeDataStore::attachBuffers(const RooArgSet& extObs) diff --git a/roofit/roofitcore/src/RooVectorDataStore.cxx b/roofit/roofitcore/src/RooVectorDataStore.cxx index 96c20887e309c..2b4ce69602989 100644 --- a/roofit/roofitcore/src/RooVectorDataStore.cxx +++ b/roofit/roofitcore/src/RooVectorDataStore.cxx @@ -279,11 +279,6 @@ RooVectorDataStore::RooVectorDataStore(RooStringView name, RooStringView title, cloneVar->attachDataStore(tds) ; } - RooVectorDataStore* vds = dynamic_cast(&tds) ; - if (vds && vds->_cache) { - _cache = new RooVectorDataStore(*vds->_cache) ; - } - loadValues(&tds,cloneVar.get(),cutRange,nStart,nStop); TRACE_CREATE; @@ -311,7 +306,6 @@ RooVectorDataStore::~RooVectorDataStore() delete elm; } - delete _cache ; TRACE_DESTROY; } @@ -372,10 +366,6 @@ const RooArgSet* RooVectorDataStore::get(Int_t index) const // Update current weight cache _currentWeightIndex = index; - if (_cache) { - _cache->get(index) ; - } - return &_vars; } @@ -750,289 +740,6 @@ void RooVectorDataStore::reset() } -//////////////////////////////////////////////////////////////////////////////// -/// Cache given RooAbsArgs: The tree is -/// given direct write access of the args internal cache -/// the args values is pre-calculated for all data points -/// in this data collection. Upon a get() call, the -/// internal cache of 'newVar' will be loaded with the -/// precalculated value and it's dirty flag will be cleared. - -void RooVectorDataStore::cacheArgs(const RooAbsArg* owner, RooArgSet& newVarSet, const RooArgSet* nset, bool skipZeroWeights) -{ - // Delete previous cache, if any - delete _cache ; - _cache = nullptr ; - - // Reorder cached elements. First constant nodes, then tracked nodes in order of dependence - - // Step 1 - split in constant and tracked - RooArgSet newVarSetCopy(newVarSet); - RooArgSet orderedArgs; - vector trackArgs; - for (const auto arg : newVarSetCopy) { - if (arg->getAttribute("ConstantExpression") && !arg->getAttribute("NOCacheAndTrack")) { - orderedArgs.add(*arg) ; - } else { - - // Explicitly check that arg depends on any of the observables, if this - // is not the case, skip it, as inclusion would result in repeated - // calculation of a function that has the same value for every event - // in the likelihood - if (arg->dependsOn(_vars) && !arg->getAttribute("NOCacheAndTrack")) { - trackArgs.push_back(arg) ; - } else { - newVarSet.remove(*arg) ; - } - } - } - - // Step 2 - reorder tracked nodes - std::sort(trackArgs.begin(), trackArgs.end(), [](RooAbsArg* left, RooAbsArg* right){ - //LM: exclude same comparison. This avoids an issue when using sort in MacOS versions - if (left == right) return false; - return right->dependsOn(*left); - }); - - // Step 3 - put back together - for (const auto trackedArg : trackArgs) { - orderedArgs.add(*trackedArg); - } - - // WVE need to prune tracking entries _below_ constant nodes as the're not needed -// std::cout << "Number of Cache-and-Tracked args are " << trackArgs.size() << std::endl ; -// std::cout << "Compound ordered cache parameters = " << std::endl ; -// orderedArgs.Print("v") ; - - checkInit() ; - - std::vector> vlist; - RooArgList cloneSet; - - for (const auto var : orderedArgs) { - - // Clone variable and attach to cloned tree - auto newVarCloneList = std::make_unique(); - RooArgSet(*var).snapshot(*newVarCloneList); - RooAbsArg* newVarClone = newVarCloneList->find(var->GetName()) ; - newVarClone->recursiveRedirectServers(_vars,false) ; - - vlist.emplace_back(std::move(newVarCloneList)); - cloneSet.add(*newVarClone) ; - } - - _cacheOwner = const_cast(owner); - RooVectorDataStore* newCache = new RooVectorDataStore("cache","cache",orderedArgs) ; - - - RooAbsArg::setDirtyInhibit(true) ; - - std::vector nsetList ; - std::vector> argObsList ; - - // Now need to attach branch buffers of clones - for (const auto arg : cloneSet) { - arg->attachToVStore(*newCache) ; - - if(nset) argObsList.emplace_back(arg->getObservables(*nset)); - else argObsList.emplace_back(arg->getVariables()); - RooArgSet* argObs = argObsList.back().get(); - - RooArgSet* normSet(nullptr) ; - const char* catNset = arg->getStringAttribute("CATNormSet") ; - if (catNset) { -// std::cout << "RooVectorDataStore::cacheArgs() cached node " << arg->GetName() << " has a normalization set specification CATNormSet = " << catNset << std::endl ; - - RooArgSet anset = RooHelpers::selectFromArgSet(nset ? *nset : RooArgSet{}, catNset); - normSet = anset.selectCommon(*argObs); - - } - const char* catCset = arg->getStringAttribute("CATCondSet") ; - if (catCset) { -// std::cout << "RooVectorDataStore::cacheArgs() cached node " << arg->GetName() << " has a conditional observable set specification CATCondSet = " << catCset << std::endl ; - - RooArgSet acset = RooHelpers::selectFromArgSet(nset ? *nset : RooArgSet{}, catCset); - argObs->remove(acset,true,true) ; - normSet = argObs ; - } - - // now construct normalization set for component from cset/nset spec -// if (normSet) { -// std::cout << "RooVectorDaraStore::cacheArgs() component " << arg->GetName() << " has custom normalization set " << *normSet << std::endl ; -// } - nsetList.push_back(normSet) ; - } - - - // Fill values of placeholder - const std::size_t numEvt = size(); - newCache->reserve(numEvt); - for (std::size_t i=0; i < numEvt; i++) { - get(i) ; - if (weight()!=0 || !skipZeroWeights) { - for (unsigned int j = 0; j < cloneSet.size(); ++j) { - auto& cloneArg = cloneSet[j]; - auto argNSet = nsetList[j]; - // WVE need to intervene here for condobs from ProdPdf - cloneArg.syncCache(argNSet ? argNSet : nset) ; - } - } - newCache->fill() ; - } - - RooAbsArg::setDirtyInhibit(false) ; - - - // Now need to attach branch buffers of original function objects - for (const auto arg : orderedArgs) { - arg->attachToVStore(*newCache) ; - - // Activate change tracking mode, if requested - if (!arg->getAttribute("ConstantExpression") && dynamic_cast(arg)) { - RealVector* rv = newCache->addReal(static_cast(arg)) ; - RooArgSet deps; - arg->getParameters(&_vars, deps); - rv->setDependents(deps) ; - - // WV lookup normalization set and associate with RealVector - // find ordinal number of arg in original list - Int_t idx = cloneSet.index(arg->GetName()) ; - - coutI(Optimization) << "RooVectorDataStore::cacheArg() element " << arg->GetName() << " has change tracking enabled on parameters " << deps << std::endl ; - rv->setNset(nsetList[idx]) ; - } - - } - - _cache = newCache ; - _cache->setDirtyProp(_doDirtyProp) ; -} - - -void RooVectorDataStore::forceCacheUpdate() -{ - if (_cache) _forcedUpdate = true ; -} - - - -//////////////////////////////////////////////////////////////////////////////// - -void RooVectorDataStore::recalculateCache( const RooArgSet *projectedArgs, Int_t firstEvent, Int_t lastEvent, Int_t stepSize, bool skipZeroWeights) -{ - if (!_cache) return ; - - std::vector tv; - tv.reserve(static_cast(_cache->_realStoreList.size() * 0.7)); // Typically, 30..60% need to be recalculated - - // Check which items need recalculation - for (const auto realVec : _cache->_realStoreList) { - if (_forcedUpdate || realVec->needRecalc()) { - tv.push_back(realVec); - realVec->_nativeReal->setOperMode(RooAbsArg::ADirty); - realVec->_nativeReal->_operMode = RooAbsArg::Auto; - } - } - _forcedUpdate = false ; - - // If no recalculations are needed stop here - if (tv.empty()) { - return; - } - - - // Refill caches of elements that require recalculation - std::unique_ptr ownedNset; - RooArgSet* usedNset = nullptr; - if (projectedArgs && !projectedArgs->empty()) { - ownedNset = std::make_unique(); - _vars.snapshot(*ownedNset, false) ; - ownedNset->remove(*projectedArgs,false,true); - usedNset = ownedNset.get(); - } else { - usedNset = &_vars ; - } - - - for (int i=firstEvent ; i_nativeReal->_valueDirty = true; - realVector->_nativeReal->getValV(realVector->_nset ? realVector->_nset : usedNset); - realVector->write(i); - } - } - } - - for (auto realVector : tv) { - realVector->_nativeReal->setOperMode(RooAbsArg::AClean); - } -} - - -//////////////////////////////////////////////////////////////////////////////// -/// Initialize cache of dataset: attach variables of cache ArgSet -/// to the corresponding TTree branches - -void RooVectorDataStore::attachCache(const RooAbsArg* newOwner, const RooArgSet& cachedVarsIn) -{ - // Only applicable if a cache exists - if (!_cache) return ; - - // Clone constructor, must connect internal storage to given new external set of variables - std::vector cacheElements(_cache->realStoreList()); - cacheElements.insert(cacheElements.end(), _cache->_realfStoreList.begin(), _cache->_realfStoreList.end()); - - for (const auto elm : cacheElements) { - auto real = static_cast(cachedVarsIn.find(elm->bufArg()->GetName())); - if (real) { - // Adjust buffer pointer - real->attachToVStore(*_cache) ; - } - } - - for (const auto catVec : _cache->_catStoreList) { - auto cat = static_cast(cachedVarsIn.find(catVec->bufArg()->GetName())); - if (cat) { - // Adjust buffer pointer - cat->attachToVStore(*_cache) ; - } - } - - _cacheOwner = const_cast(newOwner); -} - - - - -//////////////////////////////////////////////////////////////////////////////// - -void RooVectorDataStore::resetCache() -{ - delete _cache; - _cache = nullptr; - _cacheOwner = nullptr; - return ; -} - - - - - -//////////////////////////////////////////////////////////////////////////////// -/// Disabling of branches is (intentionally) not implemented in vector -/// data stores (as the doesn't result in a net saving of time) - -void RooVectorDataStore::setArgStatus(const RooArgSet& /*set*/, bool /*active*/) -{ - return ; -} - - - - //////////////////////////////////////////////////////////////////////////////// void RooVectorDataStore::attachBuffers(const RooArgSet& extObs) @@ -1157,15 +864,6 @@ RooAbsData::RealSpans RooVectorDataStore::getBatches(std::size_t first, std::siz emplace(realVec); } - if (_cache) { - for (const auto realVec : _cache->_realStoreList) { - emplace(realVec); - } - for (const auto realVec : _cache->_realfStoreList) { - emplace(realVec); - } - } - return evalData; } diff --git a/roofit/roofitcore/src/RooXYChi2Var.h b/roofit/roofitcore/src/RooXYChi2Var.h index 8c488647261f3..ca9a0c3532273 100644 --- a/roofit/roofitcore/src/RooXYChi2Var.h +++ b/roofit/roofitcore/src/RooXYChi2Var.h @@ -53,13 +53,6 @@ class RooXYChi2Var : public RooAbsOptTestStatistic { const RooNumIntConfig& binIntegratorConfig() const { return _intConfig ; } protected: - - bool allowFunctionCache() override { - // Disable function (component) caching if integration is requested as the function - // will be evaluated at coordinates other than the points in the dataset - return !_integrate ; - } - RooArgSet requiredExtraObservables() const override ; double fy() const ; diff --git a/roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.cxx b/roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.cxx deleted file mode 100644 index a6c35fff8dfad..0000000000000 --- a/roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.cxx +++ /dev/null @@ -1,171 +0,0 @@ -/* - * Project: RooFit - * Authors: - * PB, Patrick Bos, Netherlands eScience Center, p.bos@esciencecenter.nl - * - * Copyright (c) 2021, CERN - * - * Redistribution and use in source and binary forms, - * with or without modification, are permitted according to the terms - * listed in LICENSE (http://roofit.sourceforge.net/license.txt) - */ - -#include "ConstantTermsOptimizer.h" - -#include -#include // complete type for dynamic cast -#include -#include -#include - -namespace RooFit { -namespace TestStatistics { - -/** \class ConstantTermsOptimizer - * - * \brief Analyzes a function given a dataset/observables for constant terms and caches those in the dataset - * - * This optimizer should be used on a consistent combination of function (usually a pdf) and a dataset with observables. - * It then analyzes the function to find parts that can be precalculated because they are constant given the set of - * observables. These are cached inside the dataset and used in subsequent evaluations of the function on that dataset. - * The typical use case for this is inside likelihood minimization where many calls of the same pdf/dataset combination - * are made. \p norm_set must provide the normalization set of the function, which would typically be the set of - * observables in the dataset; this is used to make sure all object caches are created before analysis by evaluating the - * function on this set at the beginning of enableConstantTermsOptimization. - */ - -RooArgSet ConstantTermsOptimizer::requiredExtraObservables() -{ - // TODO: the RooAbsOptTestStatistics::requiredExtraObservables() call this code was copied - // from was overloaded for RooXYChi2Var only; implement different options when necessary - return RooArgSet(); -} - -void ConstantTermsOptimizer::enableConstantTermsOptimization(RooAbsReal *function, RooArgSet *norm_set, - RooAbsData *dataset, bool applyTrackingOpt) -{ - // Trigger create of all object caches now in nodes that have deferred object creation - // so that cache contents can be processed immediately - function->getVal(norm_set); - - // Apply tracking optimization here. Default strategy is to track components - // of RooAddPdfs and RooRealSumPdfs. If these components are a RooProdPdf - // or a RooProduct respectively, track the components of these products instead - // of the product term - RooArgSet trackNodes; - - // Add safety check here - applyTrackingOpt will only be applied if present - // dataset is constructed in terms of a RooVectorDataStore - if (applyTrackingOpt) { - if (!dynamic_cast(dataset->store())) { - oocoutW(nullptr, Optimization) - << "enableConstantTermsOptimization(function: " << function->GetName() - << ", dataset: " << dataset->GetName() - << ") WARNING Cache-and-track optimization (Optimize level 2) is only available for datasets" - << " implemented in terms of RooVectorDataStore - ignoring this option for current dataset" << std::endl; - applyTrackingOpt = false; - } - } - - if (applyTrackingOpt) { - RooArgSet branches; - function->branchNodeServerList(&branches); - for (const auto arg : branches) { - arg->setCacheAndTrackHints(trackNodes); - } - // Do not set CacheAndTrack on constant expressions - std::unique_ptr constNodes{trackNodes.selectByAttrib("Constant", true)}; - trackNodes.remove(*constNodes); - - // Set CacheAndTrack flag on all remaining nodes - trackNodes.setAttribAll("CacheAndTrack", true); - } - - // Find all nodes that depend exclusively on constant parameters - RooArgSet cached_nodes; - - function->findConstantNodes(*dataset->get(), cached_nodes); - - // Cache constant nodes with dataset - also cache entries corresponding to zero-weights in data when using - // BinnedLikelihood - // NOTE: we pass nullptr as cache-owner here, because we don't use the cacheOwner() anywhere in TestStatistics - // TODO: make sure this (nullptr) is always correct - dataset->cacheArgs(nullptr, cached_nodes, norm_set, !function->getAttribute("BinnedLikelihood")); - - // Put all cached nodes in AClean value caching mode so that their evaluate() is never called - for (const auto cacheArg : cached_nodes) { - cacheArg->setOperMode(RooAbsArg::AClean); - } - - std::unique_ptr constNodes{cached_nodes.selectByAttrib("ConstantExpressionCached", true)}; - RooArgSet actualTrackNodes(cached_nodes); - actualTrackNodes.remove(*constNodes); - if (!constNodes->empty()) { - if (constNodes->size() < 20) { - oocoutI(nullptr, Minimization) - << " The following expressions have been identified as constant and will be precalculated and cached: " - << *constNodes << std::endl; - } else { - oocoutI(nullptr, Minimization) - << " A total of " << constNodes->size() - << " expressions have been identified as constant and will be precalculated and cached." << std::endl; - } - } - if (!actualTrackNodes.empty()) { - if (actualTrackNodes.size() < 20) { - oocoutI(nullptr, Minimization) << " The following expressions will be evaluated in cache-and-track mode: " - << actualTrackNodes << std::endl; - } else { - oocoutI(nullptr, Minimization) << " A total of " << constNodes->size() - << " expressions will be evaluated in cache-and-track-mode." << std::endl; - } - } - - // Disable reading of observables that are no longer used - dataset->optimizeReadingWithCaching(*function, cached_nodes, requiredExtraObservables()); -} - -void ConstantTermsOptimizer::disableConstantTermsOptimization(RooAbsReal *function, RooArgSet *norm_set, - RooAbsData *dataset, RooArgSet *observables) -{ - // Delete the cache - dataset->resetCache(); - - // Reactivate all tree branches - dataset->setArgStatus(*dataset->get(), true); - - // Reset all nodes to ADirty - optimizeCaching(function, norm_set, dataset, observables); - - // Disable propagation of dirty state flags for observables - dataset->setDirtyProp(false); - - // _cachedNodes.removeAll(); - - // _optimized = false; -} - -void ConstantTermsOptimizer::optimizeCaching(RooAbsReal *function, RooArgSet *norm_set, RooAbsData *dataset, - RooArgSet *observables) -{ - // Trigger create of all object caches now in nodes that have deferred object creation - // so that cache contents can be processed immediately - function->getVal(norm_set); - - // Set value caching mode for all nodes that depend on any of the observables to ADirty - std::unique_ptr ownedObservables; - if (observables == nullptr) { - ownedObservables = std::unique_ptr{function->getObservables(dataset)}; - observables = ownedObservables.get(); - } - function->optimizeCacheMode(*observables); - - // Disable propagation of dirty state flags for observables - dataset->setDirtyProp(false); - - // Disable reading of observables that are not used - dataset->optimizeReadingWithCaching(*function, RooArgSet(), requiredExtraObservables()); -} - -} // namespace TestStatistics -} // namespace RooFit diff --git a/roofit/roofitcore/src/TestStatistics/LikelihoodWrapper.cxx b/roofit/roofitcore/src/TestStatistics/LikelihoodWrapper.cxx index 42914cd80a4ec..c0e7ea07fc9d4 100644 --- a/roofit/roofitcore/src/TestStatistics/LikelihoodWrapper.cxx +++ b/roofit/roofitcore/src/TestStatistics/LikelihoodWrapper.cxx @@ -77,11 +77,6 @@ void LikelihoodWrapper::synchronizeParameterSettings( { } -void LikelihoodWrapper::constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - likelihood_->constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); -} - double LikelihoodWrapper::defaultErrorLevel() const { return likelihood_->defaultErrorLevel(); diff --git a/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx b/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx index a32c95421cd8a..2ee0be304f4ad 100644 --- a/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx +++ b/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.cxx @@ -90,7 +90,7 @@ MinuitFcnGrad::MinuitFcnGrad(const std::shared_ptrgetParameters(), context), _minuitInternalX(getNDim(), 0), _minuitExternalX(getNDim(), 0) { - synchronizeParameterSettings(parameters, true); + synchronizeParameterSettings(parameters); _calculationIsClean = std::make_unique(); @@ -267,7 +267,7 @@ void MinuitFcnGrad::GradientWithPrevResult(const double *x, double *grad, double bool MinuitFcnGrad::Synchronize(std::vector ¶meters) { - bool returnee = synchronizeParameterSettings(parameters, _optConst); + bool returnee = synchronizeParameterSettings(parameters); applyToLikelihood([&](auto &l) { l.synchronizeParameterSettings(parameters); }); _gradient->synchronizeParameterSettings(parameters); diff --git a/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.h b/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.h index 74fe4f98a397d..902381c9c98aa 100644 --- a/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.h +++ b/roofit/roofitcore/src/TestStatistics/MinuitFcnGrad.h @@ -41,11 +41,6 @@ class MinuitFcnGrad : public RooAbsMinimizerFcn { bool returnsInMinuit2ParameterSpace() const { return _gradient->usesMinuitInternalValues(); } - inline void setOptimizeConstOnFunction(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override - { - applyToLikelihood([&](auto &l) { l.constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); }); - } - double operator()(const double *x) const; /// IMultiGradFunction overrides necessary for Minuit diff --git a/roofit/roofitcore/src/TestStatistics/RooAbsL.cxx b/roofit/roofitcore/src/TestStatistics/RooAbsL.cxx index 76ad278c4dfc8..adc4a1344e14f 100644 --- a/roofit/roofitcore/src/TestStatistics/RooAbsL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooAbsL.cxx @@ -13,7 +13,6 @@ #include #include "ConstantTermsOptimizer.h" #include "RooAbsData.h" -#include "RooAbsDataStore.h" // complete class for access in constOptimizeTestStatistic // for dynamic casts in init_clones: #include "RooAbsRealLValue.h" @@ -230,9 +229,6 @@ void RooAbsL::initClones(RooAbsPdf &inpdf, RooAbsData &indata) pdf_->optimizeCacheMode(*_funcObsSet); // Disable propagation of dirty state flags for observables data_->setDirtyProp(false); - - // Disable reading of observables that are not used - data_->optimizeReadingWithCaching(*pdf_, RooArgSet(), RooArgSet()); } std::unique_ptr RooAbsL::getParameters() @@ -240,50 +236,6 @@ std::unique_ptr RooAbsL::getParameters() return std::unique_ptr{pdf_->getParameters(*data_)}; } -void RooAbsL::constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - if (data_.get()->hasFilledCache() && opcode == RooAbsArg::Activate) { - opcode = RooAbsArg::ValueChange; - } - - switch (opcode) { - case RooAbsArg::Activate: - oocxcoutI((TObject *)nullptr, Optimization) - << "RooAbsL::constOptimizeTestStatistic(" << GetName() - << ") optimizing evaluation of test statistic by finding all nodes in p.d.f that depend exclusively" - << " on observables and constant parameters and precalculating their values" << std::endl; - ConstantTermsOptimizer::enableConstantTermsOptimization(pdf_.get(), normSet_.get(), data_.get(), - doAlsoTrackingOpt); - break; - - case RooAbsArg::DeActivate: - oocxcoutI((TObject *)nullptr, Optimization) - << "RooAbsL::constOptimizeTestStatistic(" << GetName() - << ") deactivating optimization of constant terms in test statistic" << std::endl; - ConstantTermsOptimizer::disableConstantTermsOptimization(pdf_.get(), normSet_.get(), data_.get()); - break; - - case RooAbsArg::ConfigChange: - oocxcoutI((TObject *)nullptr, Optimization) - << "RooAbsL::constOptimizeTestStatistic(" << GetName() - << ") one ore more parameter were changed from constant to floating or vice versa, " - << "re-evaluating constant term optimization" << std::endl; - ConstantTermsOptimizer::disableConstantTermsOptimization(pdf_.get(), normSet_.get(), data_.get()); - ConstantTermsOptimizer::enableConstantTermsOptimization(pdf_.get(), normSet_.get(), data_.get(), - doAlsoTrackingOpt); - break; - - case RooAbsArg::ValueChange: - oocxcoutI((TObject *)nullptr, Optimization) - << "RooAbsL::constOptimizeTestStatistic(" << GetName() - << ") the value of one ore more constant parameter were changed re-evaluating constant term optimization" - << std::endl; - // Request a forcible cache update of all cached nodes - data_.get()->store()->forceCacheUpdate(); - break; - } -} - std::string RooAbsL::GetName() const { std::string output("likelihood of pdf "); diff --git a/roofit/roofitcore/src/TestStatistics/RooBinnedL.cxx b/roofit/roofitcore/src/TestStatistics/RooBinnedL.cxx index 16d66d4c6a5e9..59020d94ede2e 100644 --- a/roofit/roofitcore/src/TestStatistics/RooBinnedL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooBinnedL.cxx @@ -98,10 +98,6 @@ RooBinnedL::evaluatePartition(Section bins, std::size_t /*components_begin*/, st (cachedResult_.Sum() != 0 || cachedResult_.Carry() != 0)) return cachedResult_; - // data->store()->recalculateCache(_projDeps, firstEvent, lastEvent, stepSize, (_binnedPdf?false:true)); - // TODO: check when we might need _projDeps (it seems to be mostly empty); ties in with TODO below - data_->store()->recalculateCache(nullptr, bins.begin(N_events_), bins.end(N_events_), 1, false); - ROOT::Math::KahanSum sumWeight; auto numEvalErrorsBefore = RooAbsReal::numEvalErrors(); diff --git a/roofit/roofitcore/src/TestStatistics/RooSubsidiaryL.cxx b/roofit/roofitcore/src/TestStatistics/RooSubsidiaryL.cxx index e5dfcfe21924f..885eef1ce4b03 100644 --- a/roofit/roofitcore/src/TestStatistics/RooSubsidiaryL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooSubsidiaryL.cxx @@ -67,7 +67,5 @@ ROOT::Math::KahanSum RooSubsidiaryL::evaluatePartition(RooAbsL::Section return sum; } -void RooSubsidiaryL::constOptimizeTestStatistic(RooAbsArg::ConstOpCode /*opcode*/, bool /*doAlsoTrackingOpt*/) {} - } // namespace TestStatistics } // namespace RooFit diff --git a/roofit/roofitcore/src/TestStatistics/RooSumL.cxx b/roofit/roofitcore/src/TestStatistics/RooSumL.cxx index 44faa701556e8..8646fd7fd3caa 100644 --- a/roofit/roofitcore/src/TestStatistics/RooSumL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooSumL.cxx @@ -120,12 +120,5 @@ ROOT::Math::KahanSum RooSumL::getSubsidiaryValue() return ROOT::Math::KahanSum{}; } -void RooSumL::constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - for (auto &component : components_) { - component->constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); - } -} - } // namespace TestStatistics } // namespace RooFit diff --git a/roofit/roofitcore/src/TestStatistics/RooUnbinnedL.cxx b/roofit/roofitcore/src/TestStatistics/RooUnbinnedL.cxx index ea2d95cc27a18..54f203ad2d6f0 100644 --- a/roofit/roofitcore/src/TestStatistics/RooUnbinnedL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooUnbinnedL.cxx @@ -206,7 +206,6 @@ RooUnbinnedL::evaluatePartition(Section events, std::size_t /*components_begin*/ std::tie(result, sumWeight) = computeBatchFunc(probas, data_.get(), apply_weight_squared, 1, events.begin(N_events_), events.end(N_events_)); } else { - data_->store()->recalculateCache(nullptr, events.begin(N_events_), events.end(N_events_), 1, true); std::tie(result, sumWeight) = computeScalarFunc(pdf_.get(), data_.get(), normSet_.get(), apply_weight_squared, 1, events.begin(N_events_), events.end(N_events_)); } diff --git a/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx b/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx index 296ec5f7d6d10..09871dd1f8c8e 100644 --- a/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx +++ b/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx @@ -471,7 +471,6 @@ TEST_P(SimBinnedConstrainedTest, ConstrainedAndOffset) m1.setPrintLevel(-1); m1.setOffsetting(true); - m1.optimizeConst(2); m1.minimize("Minuit2", "migrad"); diff --git a/roofit/roofitcore/test/TestStatistics/testRooAbsL.cxx b/roofit/roofitcore/test/TestStatistics/testRooAbsL.cxx index 8de165c76e104..851796f51d510 100644 --- a/roofit/roofitcore/test/TestStatistics/testRooAbsL.cxx +++ b/roofit/roofitcore/test/TestStatistics/testRooAbsL.cxx @@ -340,5 +340,3 @@ TEST_F(SimBinnedConstrainedTest, VSRooNLLVar) auto RooNLL_value = nll->getVal(); EXPECT_EQ(AbsL_value.Sum(), RooNLL_value); } - -// TODO: add tests covering all constOptimizeTestStatistic opcode cases. diff --git a/roofit/roofitcore/test/testRooProdPdf.cxx b/roofit/roofitcore/test/testRooProdPdf.cxx index 2b7d0cbd9dfed..56118169debab 100644 --- a/roofit/roofitcore/test/testRooProdPdf.cxx +++ b/roofit/roofitcore/test/testRooProdPdf.cxx @@ -22,57 +22,6 @@ #include #include -class TestProdPdf : public ::testing::TestWithParam> { -public: - TestProdPdf() : _evalBackend{RooFit::EvalBackend::Legacy()} {} - -private: - void SetUp() override - { - RooHelpers::LocalChangeMsgLevel chmsglvl{RooFit::WARNING, 0u, RooFit::NumericIntegration, true}; - - datap = std::unique_ptr{prod.generate(x, 1000)}; - a.setConstant(true); - - _optimize = std::get<0>(GetParam()); - _evalBackend = std::get<1>(GetParam()); - } - -protected: - constexpr static double bTruth = -0.5; - - RooRealVar x{"x", "x", 2., 0., 5.}; - RooRealVar a{"a", "a", -0.2, -5., 0.}; - RooRealVar b{"b", "b", bTruth, -5., 0.}; - - RooGenericPdf c1{"c1", "exp(x[0]*x[1])", {x, a}}; - RooGenericPdf c2{"c2", "exp(x[0]*x[1])", {x, b}}; - RooProdPdf prod{"mypdf", "mypdf", {c1, c2}}; - std::unique_ptr datap; - - int _optimize = 0; - RooFit::EvalBackend _evalBackend; -}; - -TEST_P(TestProdPdf, CachingOpt) -{ - RooHelpers::LocalChangeMsgLevel chmsglvl{RooFit::WARNING, 0u, RooFit::NumericIntegration, true}; - - using namespace RooFit; - prod.fitTo(*datap, Optimize(_optimize), PrintLevel(-1), _evalBackend); - EXPECT_LT(std::abs(b.getVal() - bTruth), b.getError() * 2.5) // 2.5-sigma compatibility check - << "b=" << b.getVal() << " +- " << b.getError() << " doesn't match truth value with O" << _optimize << "."; -} - -INSTANTIATE_TEST_SUITE_P(RooProdPdf, TestProdPdf, - testing::Combine(testing::Values(0, 1, 2), - testing::Values(ROOFIT_EVAL_BACKENDS)), - [](testing::TestParamInfo const ¶mInfo) { - std::stringstream ss; - ss << "opt" << std::get<0>(paramInfo.param) << std::get<1>(paramInfo.param).name(); - return ss.str(); - }); - TEST(RooProdPdf, TestGetPartIntList) { RooHelpers::LocalChangeMsgLevel chmsglvl1{RooFit::ERROR, 0u, RooFit::InputArguments, true}; diff --git a/roofit/roofitcore/test/testTestStatistics.cxx b/roofit/roofitcore/test/testTestStatistics.cxx index 47b19daa86bb4..bef2a8894621c 100644 --- a/roofit/roofitcore/test/testTestStatistics.cxx +++ b/roofit/roofitcore/test/testTestStatistics.cxx @@ -157,12 +157,12 @@ TEST_P(TestStatisticTest, IntegrateBins_SubRange) a.setVal(3.); std::unique_ptr fit1( - pdf.fitTo(*dataS, Save(), PrintLevel(-1), Optimize(0), Range("range"), _evalBackend, SumW2Error(false))); + pdf.fitTo(*dataS, Save(), PrintLevel(-1), Range("range"), _evalBackend, SumW2Error(false))); pdf.plotOn(frame.get(), LineColor(kRed), Name("standard"), Range("range"), NormRange("range")); a.setVal(3.); - std::unique_ptr fit2(pdf.fitTo(*dataS, Save(), PrintLevel(-1), Optimize(0), Range("range"), - _evalBackend, SumW2Error(false), IntegrateBins(1.E-3))); + std::unique_ptr fit2( + pdf.fitTo(*dataS, Save(), PrintLevel(-1), Range("range"), _evalBackend, SumW2Error(false), IntegrateBins(1.E-3))); pdf.plotOn(frame.get(), LineColor(kBlue), Name("highRes"), Range("range"), NormRange("range")); EXPECT_GT(std::abs(getVal("a", targetValues) - getVal("a", fit1->floatParsFinal())), @@ -208,13 +208,12 @@ TEST_P(TestStatisticTest, IntegrateBins_CustomBinning) dataS->plotOn(frame.get(), Name("data")); a.setVal(3.); - std::unique_ptr fit1( - pdf.fitTo(*dataS, Save(), PrintLevel(-1), _evalBackend, SumW2Error(false), Optimize(0))); + std::unique_ptr fit1(pdf.fitTo(*dataS, Save(), PrintLevel(-1), _evalBackend, SumW2Error(false))); pdf.plotOn(frame.get(), LineColor(kRed), Name("standard")); a.setVal(3.); std::unique_ptr fit2( - pdf.fitTo(*dataS, Save(), PrintLevel(-1), Optimize(0), _evalBackend, SumW2Error(false), IntegrateBins(1.E-3))); + pdf.fitTo(*dataS, Save(), PrintLevel(-1), _evalBackend, SumW2Error(false), IntegrateBins(1.E-3))); pdf.plotOn(frame.get(), LineColor(kBlue), Name("highRes")); EXPECT_GT(std::abs(getVal("a", targetValues) - getVal("a", fit1->floatParsFinal())), @@ -435,12 +434,12 @@ TEST(RooXYChi2Var, IntegrateLinearFunction) coefs.snapshot(savedValues); // Fit chi^2 using X and Y errors - std::unique_ptr fit1{f.chi2FitTo(dxy, YVar(y), Save(), PrintLevel(-1), Optimize(0))}; + std::unique_ptr fit1{f.chi2FitTo(dxy, YVar(y), Save(), PrintLevel(-1))}; coefs.assign(savedValues); // Alternative: fit chi^2 integrating f(x) over ranges defined by X errors, // rather than taking point at center of bin - std::unique_ptr fit2{f.chi2FitTo(dxy, YVar(y), Integrate(true), Save(), PrintLevel(-1), Optimize(0))}; + std::unique_ptr fit2{f.chi2FitTo(dxy, YVar(y), Integrate(true), Save(), PrintLevel(-1))}; // Verify that the fit result is compatible with true values within the error EXPECT_NEAR(getVal("a", fit1->floatParsFinal()), aTrue, getErr("a", fit1->floatParsFinal())); diff --git a/roofit/roostats/src/AsymptoticCalculator.cxx b/roofit/roostats/src/AsymptoticCalculator.cxx index 0a7b9a31d1ee9..e64a4f8a06bc7 100644 --- a/roofit/roostats/src/AsymptoticCalculator.cxx +++ b/roofit/roostats/src/AsymptoticCalculator.cxx @@ -381,7 +381,6 @@ double EvaluateNLL(RooStats::ModelConfig const& modelConfig, RooAbsData& data, c //LM: RooMinimizer.setPrintLevel has +1 offset - so subtract here -1 minim.setPrintLevel(minimPrintLevel-1); int status = -1; - minim.optimizeConst(2); TString minimizer = ""; // empty string to take RooMinimizer default initially TString algorithm = ROOT::Math::MinimizerOptions::DefaultMinimizerAlgo(); @@ -438,8 +437,6 @@ double EvaluateNLL(RooStats::ModelConfig const& modelConfig, RooAbsData& data, c oocoutE(nullptr,Fitting) << "FIT FAILED !- return a NaN NLL " << std::endl; val = TMath::QuietNaN(); } - - minim.optimizeConst(false); } double muTest = 0; diff --git a/roofit/roostats/src/ProfileLikelihoodCalculator.cxx b/roofit/roostats/src/ProfileLikelihoodCalculator.cxx index de7a69631be02..b6631f59612d7 100644 --- a/roofit/roostats/src/ProfileLikelihoodCalculator.cxx +++ b/roofit/roostats/src/ProfileLikelihoodCalculator.cxx @@ -169,7 +169,6 @@ RooFit::OwningPtr ProfileLikelihoodCalculator::DoMinimizeNLL(RooAb minim.setStrategy(strategy); minim.setEps(tolerance); minim.setPrintLevel(level); - minim.optimizeConst(2); // to optimize likelihood calculations minim.setEvalErrorWall(config.useEvalErrorWall); oocoutP(nullptr,Minimization) << "ProfileLikelihoodCalcultor::DoMinimizeNLL - using " << minim.minimizerType() diff --git a/roofit/roostats/src/ProfileLikelihoodTestStat.cxx b/roofit/roostats/src/ProfileLikelihoodTestStat.cxx index fe88089c7ba68..ea76640d214a0 100644 --- a/roofit/roostats/src/ProfileLikelihoodTestStat.cxx +++ b/roofit/roostats/src/ProfileLikelihoodTestStat.cxx @@ -297,8 +297,6 @@ std::unique_ptr RooStats::ProfileLikelihoodTestStat::GetMinNLL() { int level = (fPrintLevel == 0) ? -1 : fPrintLevel -2; minim.setPrintLevel(level); minim.setEps(fTolerance); - // this causes a memory leak - minim.optimizeConst(2); TString minimizer = fMinimizer; TString algorithm = ROOT::Math::MinimizerOptions::DefaultMinimizerAlgo(); if (algorithm == "Migrad") algorithm = "Minimize"; // prefer to use Minimize instead of Migrad @@ -329,5 +327,4 @@ std::unique_ptr RooStats::ProfileLikelihoodTestStat::GetMinNLL() { //how to get cov quality faster? return std::unique_ptr{minim.save()}; - //minim.optimizeConst(false); } diff --git a/roofit/xroofit/src/xRooFit.cxx b/roofit/xroofit/src/xRooFit.cxx index 2473dd4f96db4..4ec6121eb2bf1 100644 --- a/roofit/xroofit/src/xRooFit.cxx +++ b/roofit/xroofit/src/xRooFit.cxx @@ -479,8 +479,6 @@ std::shared_ptr xRooFit::defaultNLLOptions() delete l; }); sDefaultNLLOptions->Add(RooFit::Offset().Clone()); - // disable const-optimization at the construction step ... can happen in the minimization though - sDefaultNLLOptions->Add(RooFit::Optimize(0).Clone()); return sDefaultNLLOptions; } @@ -595,10 +593,6 @@ class ProgressMonitor : public RooAbsReal { { fFunc->printMultiline(os, contents, verbose, indent); } - void constOptimizeTestStatistic(ConstOpCode opcode, bool doAlsoTrackingOpt) override - { - fFunc->constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); - } double evaluate() const override { @@ -989,26 +983,6 @@ std::shared_ptr xRooFit::minimize(RooAbsReal &nll, int status = 0; - int constOptimize = 2; - _minimizer.fitter()->Config().MinimizerOptions().ExtraOptions()->GetValue("OptimizeConst", constOptimize); - if (constOptimize) { - _minimizer.optimizeConst(constOptimize); - // for safety force a refresh of the cache (and tracking) in the nll - // DO NOT do a ConfigChange ... this is just a deactivate-reactivate of caching - // but it seems like doing this breaks the const optimization and function is badly behaved - // so once its turned on never turn it off. - // nll.constOptimizeTestStatistic(RooAbsArg::ConfigChange, constOptimize>1 /* do tracking too if >1 */); // - // trigger a re-evaluate of which nodes to cache-and-track - // the next line seems safe to do but wont bother doing it because not bothering with above - // need to understand why turning the cache off and on again breaks it?? - // nll.constOptimizeTestStatistic(RooAbsArg::ValueChange, constOptimize>1); // update the cache values -- is - // this needed?? - } else { - // disable const optimization - // warning - if the nll was previously activated then it seems like deactivating may break it. - nll.constOptimizeTestStatistic(RooAbsArg::DeActivate); - } - int sIdx = -1; TString minim = _minimizer.fitter()->Config().MinimizerType(); TString algo = _minimizer.fitter()->Config().MinimizerAlgoType();