From 810f9662bf1e3735920ac6a5b90ef241bef466b5 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Wed, 8 Apr 2026 09:59:42 +0200 Subject: [PATCH] [RF] Remove all code related to constant term optimization This is one of the most complicated aspects of RooFit, which was deprecated in 6.40 and scheduled for removal in 6.42. It amounted to over 1 percent of the RooFitCore source code, and is now superseded by the vectorized evaluation backend, which does the caching of vector buffers transparently in the RooFit evaluator. --- README/ReleaseNotes/v642/index.md | 25 +- roofit/doc/developers/test_statistics.md | 31 -- roofit/histfactory/test/testHistFactory.cxx | 131 ++++---- roofit/roofit/inc/RooLagrangianMorphFunc.h | 2 - roofit/roofit/src/RooLagrangianMorphFunc.cxx | 16 - roofit/roofit/src/RooMomentMorph.cxx | 2 +- .../vectorisedPDFs/VectorisedPDFTests.cxx | 2 +- .../test/vectorisedPDFs/testGaussBinned.cxx | 2 +- roofit/roofitcore/CMakeLists.txt | 2 - roofit/roofitcore/inc/RooAbsAnaConvPdf.h | 1 - roofit/roofitcore/inc/RooAbsArg.h | 22 -- roofit/roofitcore/inc/RooAbsCache.h | 1 - roofit/roofitcore/inc/RooAbsCacheElement.h | 3 +- roofit/roofitcore/inc/RooAbsCategory.h | 1 - roofit/roofitcore/inc/RooAbsData.h | 11 - roofit/roofitcore/inc/RooAbsDataStore.h | 13 - roofit/roofitcore/inc/RooAbsReal.h | 1 - roofit/roofitcore/inc/RooAddPdf.h | 2 - roofit/roofitcore/inc/RooCompositeDataStore.h | 13 - roofit/roofitcore/inc/RooEvaluatorWrapper.h | 3 - .../RooFit/TestStatistics/LikelihoodWrapper.h | 2 - .../inc/RooFit/TestStatistics/RooAbsL.h | 7 - .../RooFit/TestStatistics/RooSubsidiaryL.h | 2 - .../inc/RooFit/TestStatistics/RooSumL.h | 2 - .../inc/RooFit/TestStatistics/RooUnbinnedL.h | 6 - roofit/roofitcore/inc/RooObjCacheManager.h | 3 +- roofit/roofitcore/inc/RooProdPdf.h | 2 - roofit/roofitcore/inc/RooProduct.h | 2 - roofit/roofitcore/inc/RooRealSumFunc.h | 2 - roofit/roofitcore/inc/RooRealSumPdf.h | 3 - roofit/roofitcore/inc/RooStringVar.h | 1 - roofit/roofitcore/inc/RooTreeDataStore.h | 9 - roofit/roofitcore/inc/RooVectorDataStore.h | 28 -- roofit/roofitcore/src/FitHelpers.cxx | 22 +- roofit/roofitcore/src/RooAbsAnaConvPdf.cxx | 16 - roofit/roofitcore/src/RooAbsArg.cxx | 96 ------ roofit/roofitcore/src/RooAbsCacheElement.cxx | 13 - roofit/roofitcore/src/RooAbsCategory.cxx | 12 - roofit/roofitcore/src/RooAbsData.cxx | 122 +------- roofit/roofitcore/src/RooAbsDataStore.cxx | 4 - roofit/roofitcore/src/RooAbsMinimizerFcn.cxx | 66 +--- roofit/roofitcore/src/RooAbsMinimizerFcn.h | 11 +- .../roofitcore/src/RooAbsOptTestStatistic.cxx | 219 ------------- .../roofitcore/src/RooAbsOptTestStatistic.h | 9 - roofit/roofitcore/src/RooAbsReal.cxx | 12 - roofit/roofitcore/src/RooAbsTestStatistic.cxx | 28 -- roofit/roofitcore/src/RooAbsTestStatistic.h | 5 - roofit/roofitcore/src/RooAddPdf.cxx | 10 - .../roofitcore/src/RooCompositeDataStore.cxx | 85 ----- roofit/roofitcore/src/RooDataSet.cxx | 2 - roofit/roofitcore/src/RooMinimizer.cxx | 3 +- roofit/roofitcore/src/RooMinimizerFcn.cxx | 5 - roofit/roofitcore/src/RooMinimizerFcn.h | 2 - roofit/roofitcore/src/RooNLLVar.cxx | 4 - roofit/roofitcore/src/RooObjCacheManager.cxx | 19 -- roofit/roofitcore/src/RooProdPdf.cxx | 30 -- roofit/roofitcore/src/RooProduct.cxx | 15 - roofit/roofitcore/src/RooRealIntegral.cxx | 25 +- roofit/roofitcore/src/RooRealMPFE.cxx | 38 --- roofit/roofitcore/src/RooRealMPFE.h | 3 +- roofit/roofitcore/src/RooRealSumFunc.cxx | 6 - roofit/roofitcore/src/RooRealSumPdf.cxx | 19 -- roofit/roofitcore/src/RooStringVar.cxx | 12 - roofit/roofitcore/src/RooTreeDataStore.cxx | 137 -------- roofit/roofitcore/src/RooVectorDataStore.cxx | 294 ------------------ .../TestStatistics/ConstantTermsOptimizer.cxx | 171 ---------- .../TestStatistics/ConstantTermsOptimizer.h | 38 --- .../src/TestStatistics/LikelihoodJob.cxx | 4 - .../src/TestStatistics/LikelihoodSerial.cxx | 4 - .../src/TestStatistics/LikelihoodWrapper.cxx | 5 - .../src/TestStatistics/MinuitFcnGrad.cxx | 4 +- .../src/TestStatistics/MinuitFcnGrad.h | 5 - .../roofitcore/src/TestStatistics/RooAbsL.cxx | 49 --- .../src/TestStatistics/RooBinnedL.cxx | 4 - .../src/TestStatistics/RooSubsidiaryL.cxx | 2 - .../roofitcore/src/TestStatistics/RooSumL.cxx | 7 - .../src/TestStatistics/RooUnbinnedL.cxx | 34 -- .../testLikelihoodGradientJob.cxx | 1 - .../test/TestStatistics/testRooAbsL.cxx | 2 - roofit/roofitcore/test/testRooProdPdf.cxx | 51 --- roofit/roofitcore/test/testTestStatistics.cxx | 11 +- .../RooStats/MaxLikelihoodEstimateTestStat.h | 1 - roofit/roostats/src/AsymptoticCalculator.cxx | 3 - .../src/ProfileLikelihoodCalculator.cxx | 1 - .../src/ProfileLikelihoodTestStat.cxx | 3 - roofit/xroofit/src/xRooFit.cxx | 32 -- roofit/xroofit/src/xRooNLLVar.cxx | 14 +- .../roofit/roofit/rf712_lagrangianmorphfit.py | 2 +- 88 files changed, 103 insertions(+), 2042 deletions(-) delete mode 100644 roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.cxx delete mode 100644 roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.h diff --git a/README/ReleaseNotes/v642/index.md b/README/ReleaseNotes/v642/index.md index e83165d9f748d..85c183fbbd094 100644 --- a/README/ReleaseNotes/v642/index.md +++ b/README/ReleaseNotes/v642/index.md @@ -136,12 +136,31 @@ the cut instead of being selected based on `sqrt(abs(x))`. * The `RooMinimizer::Strategy` enum has been removed. It named the Minuit strategies that are usually referred to just by integers, but caused confusion because it didn't include the unnamed "Strategy 3". Since people usually set the strategy with integer values anyway, it was decided that the simplest solution to avoid the confusion was simply to remove the `RooMinimizer::Strategy` enum -### Removal of the the constant term optimization for legacy test statistic classes +### Removal of the constant term optimization for legacy test statistic classes -The **RooFit::Optimize()** option (constant term optimization) has been deprecated in ROOT 6.40 its functionality was now removed. +The **RooFit::Optimize()** option (constant term optimization) was deprecated in ROOT 6.40, and its functionality is now removed. The `RooFit::Optimize()` and `RooMinimizer::optimizeConst()` methods are kept for API consistency across ROOT versions, but they have no effect anymore. -This option only affected the `legacy` evaluation backend. +In practice this option only affected the `legacy` evaluation backend. + +Together with the mechanism itself, the public interfaces that only existed to +drive it were removed. Code that called or overrode any of the following needs +to be adapted: + + * `RooAbsArg::constOptimizeTestStatistic()`, `RooAbsArg::findConstantNodes()`, + `RooAbsArg::setCacheAndTrackHints()`, `RooAbsArg::canNodeBeCached()` and the + `RooAbsArg::ConstOpCode` and `RooAbsArg::CacheMode` enums. + * `RooAbsData::cacheArgs()`, `resetCache()`, `setArgStatus()`, `attachCache()`, + `optimizeReadingWithCaching()`, `allClientsCached()` and `hasFilledCache()`, + together with the corresponding `RooAbsDataStore` interface (`cacheArgs()`, + `cacheOwner()`, `attachCache()`, `setArgStatus()`, `resetCache()`, + `cachedVars()`, `recalculateCache()` and `forceCacheUpdate()`). Classes + deriving from `RooAbsDataStore` no longer need to implement them. + * `RooFit::TestStatistics::RooAbsL::constOptimizeTestStatistic()` and + `RooFit::TestStatistics::LikelihoodWrapper::constOptimizeTestStatistic()`. + +Since the cache-and-track hints are gone, the `"CacheAndTrack"`, +`"NOCacheAndTrack"` and `"NeverConstant"` attributes no longer have any effect. The default vectorized CPU evaluation backend (introduced in ROOT 6.32) already performs these optimizations automatically and is not affected by this change. Users are strongly encouraged to switch to the vectorized CPU backend if they are still using the legacy backend. diff --git a/roofit/doc/developers/test_statistics.md b/roofit/doc/developers/test_statistics.md index efc3209b7aa32..ab46ff20deca3 100644 --- a/roofit/doc/developers/test_statistics.md +++ b/roofit/doc/developers/test_statistics.md @@ -147,37 +147,6 @@ Make sure to also try the alternative calculation backends from the `RooFit::Eva It will depend on your use-case which combination of options gives you the best computational performance. -## Constant term optimization -The `RooAbsTestStatistic` based classes not only combine statistics and calculation, but also constant term optimization routines. -These can be run on PDFs and datasets before starting a fit. -They search the calculation graph for parts that are independent of the fit parameters, precalculates them, and adds them to (a clone of) the dataset so that these values can be used during calculation. - -In `RooFit::TestStatistics`, we separated this functionality out into the `ConstantTermsOptimizer` class. -In fact, it is not so much a class, as it is a collection of static functions that can be applied to any combination of pdf and dataset. -This class does essentially the same as `constOptimizeTestStatistic` did on a `RooNLLVar`, except that it has been factored out into a separate class. - -### Usage example: apply constant term optimization on pdf and dataset inside a likelihood -Applying the default `ConstantTermsOptimizer` optimization routines on the pdf and dataset inside a `RooAbsL` likelihood is as simple as: - -``` {.cpp} -likelihood.constOptimizeTestStatistic(); -``` -This applies constant term optimization to the cloned pdf and dataset inside the likelihood object. -It will not modify anything outside of the likelihood. - -Optimization can also be activated through the minimizer, which may be more familiar to most users. -Given the `RooMinimizer` object `m` as defined in the example above, we can do: -``` {.cpp} -m.optimizeConst(2); -``` - -For the adventurous user, it is also possible to apply constant term optimization to a pdf and dataset directly without needing a likelihood object, e.g. given some `RooArgSet` set of observables `normSet`: -``` {.cpp} -bool applyTrackingOpt = true; -ConstantTermsOptimizer::enableConstantTermsOptimization(&pdf, &normSet, dataset, applyTrackingOpt); -``` -We refer to RooFit documentation for more about "tracking optimization" which can be enabled or disabled using the final boolean parameter. - ## Load balancing options A number of calculation strategy options are available to tune load balancing. diff --git a/roofit/histfactory/test/testHistFactory.cxx b/roofit/histfactory/test/testHistFactory.cxx index 8bb9eabf62653..594d7b73395b0 100644 --- a/roofit/histfactory/test/testHistFactory.cxx +++ b/roofit/histfactory/test/testHistFactory.cxx @@ -595,82 +595,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/inc/RooLagrangianMorphFunc.h b/roofit/roofit/inc/RooLagrangianMorphFunc.h index 6eef5b4db6622..aef8ae28260d0 100644 --- a/roofit/roofit/inc/RooLagrangianMorphFunc.h +++ b/roofit/roofit/inc/RooLagrangianMorphFunc.h @@ -117,8 +117,6 @@ class RooLagrangianMorphFunc : public RooAbsReal { const char *rangeName = nullptr) const override; double analyticalIntegralWN(Int_t code, const RooArgSet *normSet, const char *rangeName = nullptr) const override; void printMetaArgs(std::ostream &os) const override; - RooAbsArg::CacheMode canNodeBeCached() const override; - void setCacheAndTrackHints(RooArgSet &) override; void insert(RooWorkspace *ws); diff --git a/roofit/roofit/src/RooLagrangianMorphFunc.cxx b/roofit/roofit/src/RooLagrangianMorphFunc.cxx index bfaf0259722c5..b7a5807f3d346 100644 --- a/roofit/roofit/src/RooLagrangianMorphFunc.cxx +++ b/roofit/roofit/src/RooLagrangianMorphFunc.cxx @@ -2940,22 +2940,6 @@ void RooLagrangianMorphFunc::printMetaArgs(std::ostream &os) const //////////////////////////////////////////////////////////////////////////////// /// Retrieve the matrix of coefficients -RooAbsArg::CacheMode RooLagrangianMorphFunc::canNodeBeCached() const -{ - return this->getFunc()->canNodeBeCached(); -} - -//////////////////////////////////////////////////////////////////////////////// -/// Retrieve the matrix of coefficients - -void RooLagrangianMorphFunc::setCacheAndTrackHints(RooArgSet &arg) -{ - this->getFunc()->setCacheAndTrackHints(arg); -} - -//////////////////////////////////////////////////////////////////////////////// -/// Retrieve the matrix of coefficients - TMatrixD RooLagrangianMorphFunc::getMatrix() const { auto cache = getCache(); diff --git a/roofit/roofit/src/RooMomentMorph.cxx b/roofit/roofit/src/RooMomentMorph.cxx index 4326c899256f8..dd83dedbded18 100644 --- a/roofit/roofit/src/RooMomentMorph.cxx +++ b/roofit/roofit/src/RooMomentMorph.cxx @@ -35,7 +35,7 @@ /// coverity[UNINIT_CTOR] RooMomentMorph::RooMomentMorph() - : _cacheMgr(this,10,true,true) + : _cacheMgr(this, 10, true, true) { } diff --git a/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx b/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx index 282865e692304..1a9ccfac3d162 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 6e7429abe6164..d8143917c8073 100644 --- a/roofit/roofitcore/CMakeLists.txt +++ b/roofit/roofitcore/CMakeLists.txt @@ -17,7 +17,6 @@ if(roofit_multiprocess) ) set(RooFitMPTestStatisticsHeaders src/TestStatistics/LikelihoodGradientJob.h - src/TestStatistics/ConstantTermsOptimizer.h src/TestStatistics/LikelihoodSerial.h src/TestStatistics/MinuitFcnGrad.h src/TestStatistics/LikelihoodJob.h @@ -454,7 +453,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/RooAbsAnaConvPdf.h b/roofit/roofitcore/inc/RooAbsAnaConvPdf.h index e2667488c6d4c..5963cc3e801aa 100644 --- a/roofit/roofitcore/inc/RooAbsAnaConvPdf.h +++ b/roofit/roofitcore/inc/RooAbsAnaConvPdf.h @@ -67,7 +67,6 @@ class RooAbsAnaConvPdf : public RooAbsPdf { bool isDirectGenSafe(const RooAbsArg& arg) const override ; - void setCacheAndTrackHints(RooArgSet&) override ; RooAbsGenContext* genContext(const RooArgSet &vars, const RooDataSet *prototype=nullptr, const RooArgSet* auxProto=nullptr, bool verbose= false) const override ; diff --git a/roofit/roofitcore/inc/RooAbsArg.h b/roofit/roofitcore/inc/RooAbsArg.h index ab6a659165f71..b53f27a8185b6 100644 --- a/roofit/roofitcore/inc/RooAbsArg.h +++ b/roofit/roofitcore/inc/RooAbsArg.h @@ -308,17 +308,6 @@ class RooAbsArg : public TNamed, public RooPrintable { return false; } - enum ConstOpCode { - Activate = 0, - DeActivate = 1, - ConfigChange = 2, - ValueChange = 3 - }; - enum CacheMode { - Always = 0, - NotAdvised = 1, - Never = 2 - }; enum OperMode { Auto = 0, AClean = 1, @@ -336,16 +325,6 @@ class RooAbsArg : public TNamed, public RooPrintable { virtual void optimizeCacheMode(const RooArgSet &observables); virtual void optimizeCacheMode(const RooArgSet &observables, RooArgSet &optNodes, RooLinkedList &processedNodes); - // Find constant terms in expression - 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*/) {}; - // Dirty state accessor inline bool isShapeDirty() const { @@ -605,7 +584,6 @@ class RooAbsArg : public TNamed, public RooPrintable { /// Attach this argument to the data store such that it reads data from there. void attachToStore(RooAbsDataStore &store); - virtual void setTreeBranchStatus(TTree &t, bool active) = 0; virtual void fillTreeBranch(TTree &t) = 0; TString cleanBranchName() const; diff --git a/roofit/roofitcore/inc/RooAbsCache.h b/roofit/roofitcore/inc/RooAbsCache.h index eee94d125beeb..d2f36b57a3768 100644 --- a/roofit/roofitcore/inc/RooAbsCache.h +++ b/roofit/roofitcore/inc/RooAbsCache.h @@ -49,7 +49,6 @@ class RooAbsCache { virtual void optimizeCacheMode(const RooArgSet&, RooArgSet&, RooLinkedList& ) {} /// Interface for constant term node finding calls. - virtual void findConstantNodes(const RooArgSet&, RooArgSet& , RooLinkedList&) {} /// Interface for printing of cache guts in tree mode printing. virtual void printCompactTreeHook(std::ostream&, const char *) {} diff --git a/roofit/roofitcore/inc/RooAbsCacheElement.h b/roofit/roofitcore/inc/RooAbsCacheElement.h index 7682d511ce4d2..10c3acf0618d3 100644 --- a/roofit/roofitcore/inc/RooAbsCacheElement.h +++ b/roofit/roofitcore/inc/RooAbsCacheElement.h @@ -36,12 +36,11 @@ class RooAbsCacheElement { // Destructor } ; - enum Action { OperModeChange,OptimizeCaching,FindConstantNodes } ; + enum Action { OperModeChange,OptimizeCaching } ; virtual RooArgList containedArgs(Action) = 0 ; /// Interface for changes of operation mode. virtual void operModeHook(RooAbsArg::OperMode) { } virtual void optimizeCacheMode(const RooArgSet& obs, RooArgSet& optNodes, RooLinkedList& processedNodes) ; - virtual void findConstantNodes(const RooArgSet& obs, RooArgSet& cacheList, RooLinkedList& processedNodes) ; void setOwner(RooAbsArg* owner) { // Store pointer to owner diff --git a/roofit/roofitcore/inc/RooAbsCategory.h b/roofit/roofitcore/inc/RooAbsCategory.h index 1244e2423a9b3..1ba9b29e2901d 100644 --- a/roofit/roofitcore/inc/RooAbsCategory.h +++ b/roofit/roofitcore/inc/RooAbsCategory.h @@ -207,7 +207,6 @@ class RooAbsCategory : public RooAbsArg { void setCachedValue(double value, bool notifyClients = true) final; void attachToTree(TTree& t, Int_t bufSize=32000) override ; void attachToVStore(RooVectorDataStore& vstore) override ; - void setTreeBranchStatus(TTree& t, bool active) override ; void fillTreeBranch(TTree& t) override ; RooCatType* retrieveLegacyState(value_type index) const; diff --git a/roofit/roofitcore/inc/RooAbsData.h b/roofit/roofitcore/inc/RooAbsData.h index 7a8bd894d6828..78d4868c0d45a 100644 --- a/roofit/roofitcore/inc/RooAbsData.h +++ b/roofit/roofitcore/inc/RooAbsData.h @@ -49,7 +49,6 @@ class TH1; class TH2F; namespace RooFit::TestStatistics { class RooAbsL; -struct ConstantTermsOptimizer; } // namespace RooFit::TestStatistics @@ -257,7 +256,6 @@ class RooAbsData : public TNamed, public RooPrintable { void RecursiveRemove(TObject *obj) override; - bool hasFilledCache() const ; void addOwnedComponent(const char* idxlabel, RooAbsData& data) ; @@ -304,9 +302,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"; @@ -337,15 +332,9 @@ class RooAbsData : public TNamed, public RooPrintable { // Constant term optimizer interface friend class RooAbsOptTestStatistic ; - friend struct RooFit::TestStatistics::ConstantTermsOptimizer; // 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..0367d0245fbbd 100644 --- a/roofit/roofitcore/inc/RooAbsDataStore.h +++ b/roofit/roofitcore/inc/RooAbsDataStore.h @@ -121,29 +121,16 @@ 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 ; } virtual void checkInit() const {} ; - virtual bool hasFilledCache() const { return false ; } - virtual void dump() {} 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/RooAbsReal.h b/roofit/roofitcore/inc/RooAbsReal.h index 80d1053ed1bf5..0e3299bfbe6d7 100644 --- a/roofit/roofitcore/inc/RooAbsReal.h +++ b/roofit/roofitcore/inc/RooAbsReal.h @@ -457,7 +457,6 @@ class RooAbsReal : public RooAbsArg { void copyCache(const RooAbsArg* source, bool valueOnly=false, bool setValDirty=true) override ; void attachToTree(TTree& t, Int_t bufSize=32000) override ; void attachToVStore(RooVectorDataStore& vstore) override ; - void setTreeBranchStatus(TTree& t, bool active) override ; void fillTreeBranch(TTree& t) override ; struct PlotOpt { diff --git a/roofit/roofitcore/inc/RooAddPdf.h b/roofit/roofitcore/inc/RooAddPdf.h index 16323c042522e..591f2d990999b 100644 --- a/roofit/roofitcore/inc/RooAddPdf.h +++ b/roofit/roofitcore/inc/RooAddPdf.h @@ -89,8 +89,6 @@ class RooAddPdf : public RooAbsPdf { void printMetaArgs(std::ostream& os) const override; - CacheMode canNodeBeCached() const override { return RooAbsArg::NotAdvised ; }; - void setCacheAndTrackHints(RooArgSet&) override; std::unique_ptr compileForNormSet(RooArgSet const &normSet, RooFit::Detail::CompileContext & ctx) const override; 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; ///applyWeightSquared(flag); } - /// 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..743d4ac2e6ff6 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/LikelihoodWrapper.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/LikelihoodWrapper.h @@ -16,7 +16,6 @@ #include "RooFit/TestStatistics/SharedOffset.h" #include "RooArgSet.h" -#include "RooAbsArg.h" // enum ConstOpCode #include #include @@ -94,7 +93,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..6ca5711fad372 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/RooAbsL.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/RooAbsL.h @@ -14,7 +14,6 @@ #define ROOT_ROOFIT_TESTSTATISTICS_RooAbsL #include "RooArgSet.h" -#include "RooAbsArg.h" // enum ConstOpCode #include "RooAbsPdf.h" #include "Math/Util.h" // KahanSum @@ -109,12 +108,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/RooFit/TestStatistics/RooUnbinnedL.h b/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h index c0f79515a6822..18b1450cf82ce 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h @@ -41,7 +41,6 @@ class RooUnbinnedL : public RooAbsL { ROOT::Math::KahanSum evaluatePartition(Section events, std::size_t components_begin, std::size_t components_end) override; - void constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) override; std::string GetClassName() const override { return "RooUnbinnedL"; } @@ -50,11 +49,6 @@ class RooUnbinnedL : public RooAbsL { mutable bool _first = true; /// paramTracker_; Section lastSection_ = {0, 0}; // used for cache together with the parameter tracker - // Last event section the dataset cache was recalculated for. The - // cache-and-track optimization tracks staleness globally per cached branch, - // but only refreshes the requested event range, so evaluating a different - // section requires forcing a cache update (see evaluatePartition). - Section lastCacheSection_ = {0, 0}; mutable ROOT::Math::KahanSum cachedResult_{0.}; std::shared_ptr evaluator_; ///> _vectorBuffers; // used for preserving resources in batched evaluation diff --git a/roofit/roofitcore/inc/RooObjCacheManager.h b/roofit/roofitcore/inc/RooObjCacheManager.h index 5cfe563248958..aca7a7c6a8876 100644 --- a/roofit/roofitcore/inc/RooObjCacheManager.h +++ b/roofit/roofitcore/inc/RooObjCacheManager.h @@ -38,7 +38,6 @@ class RooObjCacheManager : public RooCacheManager { void operModeHook() override ; void optimizeCacheMode(const RooArgSet& /*obs*/, RooArgSet& /*optSet*/, RooLinkedList& /*processedNodes*/) override ; void printCompactTreeHook(std::ostream&, const char *) override ; - void findConstantNodes(const RooArgSet& /*obs*/, RooArgSet& /*cacheList*/, RooLinkedList& /*processedNodes*/) override ; void insertObjectHook(RooAbsCacheElement&) override ; @@ -52,7 +51,7 @@ class RooObjCacheManager : public RooCacheManager { protected: bool _clearOnRedirect ; - bool _allowOptimize ; + bool _allowOptimize ; ///< No longer used, but kept because it is a persistent member (schema evolution) bool _optCacheModeSeen ; /// createCacheElem(const RooArgSet* nset, const RooArgSet* iset, const char* isetRangeName=nullptr) const; diff --git a/roofit/roofitcore/inc/RooProduct.h b/roofit/roofitcore/inc/RooProduct.h index a23d61914d53b..5704cd44793c8 100644 --- a/roofit/roofitcore/inc/RooProduct.h +++ b/roofit/roofitcore/inc/RooProduct.h @@ -60,8 +60,6 @@ class RooProduct : public RooAbsReal { std::list* plotSamplingHint(RooAbsRealLValue& /*obs*/, double /*xlo*/, double /*xhi*/) const override ; bool isBinnedDistribution(const RooArgSet& obs) const override ; - CacheMode canNodeBeCached() const override { return RooAbsArg::NotAdvised ; } ; - void setCacheAndTrackHints(RooArgSet&) override ; protected: diff --git a/roofit/roofitcore/inc/RooRealSumFunc.h b/roofit/roofitcore/inc/RooRealSumFunc.h index f43de2ae1dd03..debe93977e85a 100644 --- a/roofit/roofitcore/inc/RooRealSumFunc.h +++ b/roofit/roofitcore/inc/RooRealSumFunc.h @@ -54,8 +54,6 @@ class RooRealSumFunc : public RooAbsReal { static void setFloorGlobal(bool flag) { _doFloorGlobal = flag; } static bool getFloorGlobal() { return _doFloorGlobal; } - CacheMode canNodeBeCached() const override { return RooAbsArg::NotAdvised; }; - void setCacheAndTrackHints(RooArgSet &) override; std::unique_ptr compileForNormSet(RooArgSet const &normSet, RooFit::Detail::CompileContext & ctx) const override; diff --git a/roofit/roofitcore/inc/RooRealSumPdf.h b/roofit/roofitcore/inc/RooRealSumPdf.h index 740c4aa6176f0..2018dcfed2783 100644 --- a/roofit/roofitcore/inc/RooRealSumPdf.h +++ b/roofit/roofitcore/inc/RooRealSumPdf.h @@ -64,8 +64,6 @@ class RooRealSumPdf : public RooAbsPdf { static void setFloorGlobal(bool flag) { _doFloorGlobal = flag ; } static bool getFloorGlobal() { return _doFloorGlobal ; } - CacheMode canNodeBeCached() const override { return RooAbsArg::NotAdvised ; } ; - void setCacheAndTrackHints(RooArgSet&) override ; std::unique_ptr compileForNormSet(RooArgSet const &normSet, RooFit::Detail::CompileContext & ctx) const override; @@ -132,7 +130,6 @@ class RooRealSumPdf : public RooAbsPdf { static void printMetaArgs(RooArgList const& funcList, RooArgList const& coefList, std::ostream& os); - static void setCacheAndTrackHints(RooArgList const& funcList, RooArgSet& trackNodes); inline void setExtended(bool extended) { _extended = extended; } diff --git a/roofit/roofitcore/inc/RooStringVar.h b/roofit/roofitcore/inc/RooStringVar.h index 36ba5675667a0..2dad8e1084e96 100644 --- a/roofit/roofitcore/inc/RooStringVar.h +++ b/roofit/roofitcore/inc/RooStringVar.h @@ -69,7 +69,6 @@ class RooStringVar final : public RooAbsArg { void attachToTree(TTree& t, Int_t bufSize=32000) override; void attachToVStore(RooVectorDataStore&) override { } void fillTreeBranch(TTree& t) override; - void setTreeBranchStatus(TTree& t, bool active) override; private: std::string _string; 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; @@ -182,13 +170,6 @@ class RooVectorDataStore : public RooAbsDataStore { _extSumW2Array = arraySumW2 ; } - void setDirtyProp(bool flag) override { - _doDirtyProp = flag ; - if (_cache) { - _cache->setDirtyProp(flag) ; - } - } - const RooArgSet& row() { return _varsww ; } class RealVector { @@ -559,10 +540,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 +560,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"); @@ -676,8 +673,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) @@ -712,8 +707,6 @@ std::unique_ptr minimize(RooAbsReal &pdf, RooAbsReal &nll, RooAbsD ret->setCovQual(corrCovQual); } - if (cfg.optConst) - m.optimizeConst(0); return ret; } @@ -738,8 +731,7 @@ 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.defineInt("cloneData", "CloneData", 0, 0); pc.defineSet("projDepSet", "ProjectedObservables", 0, nullptr); pc.defineSet("cPars", "Constrain", 0, nullptr); pc.defineSet("glObs", "GlobalObservables", 0, nullptr); @@ -800,15 +792,9 @@ 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 - if (cloneData == 2) { - cloneData = optConst; - } - if (pc.hasProcessed("Range")) { double rangeLo = pc.getDouble("rangeLo"); double rangeHi = pc.getDouble("rangeHi"); @@ -997,10 +983,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); } @@ -1225,7 +1207,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,EvalBackend"; auto createChi2DataSetCmdArgs = "YVar,Integrate,RangeWithName,NumCPU,Verbose"; nllCmdListString += isDataHist ? createChi2DataHistCmdArgs : createChi2DataSetCmdArgs; diff --git a/roofit/roofitcore/src/RooAbsAnaConvPdf.cxx b/roofit/roofitcore/src/RooAbsAnaConvPdf.cxx index d73c0f0edbf94..3de92c7878e8d 100644 --- a/roofit/roofitcore/src/RooAbsAnaConvPdf.cxx +++ b/roofit/roofitcore/src/RooAbsAnaConvPdf.cxx @@ -139,7 +139,6 @@ RooAbsAnaConvPdf::RooAbsAnaConvPdf(const char *name, const char *title, const Ro _coefNormMgr(this, 10), _codeReg(10) { - _model->setAttribute("NOCacheAndTrack"); } @@ -158,7 +157,6 @@ RooAbsAnaConvPdf::RooAbsAnaConvPdf(const RooAbsAnaConvPdf &other, const char *na { // Copy constructor if (_model) { - _model->setAttribute("NOCacheAndTrack"); } other._basisList.snapshot(_basisList); } @@ -267,7 +265,6 @@ Int_t RooAbsAnaConvPdf::declareBasis(const char* expression, const RooArgList& p auto basisFunc = std::make_unique(basisName, expression, basisArgs); basisFunc->setAttribute("RooWorkspace::Recycle") ; - basisFunc->setAttribute("NOCacheAndTrack") ; basisFunc->setOperMode(operMode()) ; // Instantiate resModel x basisFunc convolution @@ -319,7 +316,6 @@ bool RooAbsAnaConvPdf::changeModel(const RooResolutionModel& newModel) } _model = static_cast(newModel.clone(newModel.GetName())); _ownModel = true; - _model->setAttribute("NOCacheAndTrack"); return false ; } @@ -733,18 +729,6 @@ void RooAbsAnaConvPdf::printMultiline(ostream& os, Int_t contents, bool verbose, } -/////////////////////////////////////////////////////////////////////////////// -/// Label OK'ed components with cache-and-track -void RooAbsAnaConvPdf::setCacheAndTrackHints(RooArgSet& trackNodes) -{ - for (auto const* carg : static_range_cast(_convSet)) { - if (carg->canNodeBeCached()==Always) { - trackNodes.add(*carg) ; - //cout << "tracking node RooAddPdf component " << carg->ClassName() << "::" << carg->GetName() << std::endl ; - } - } -} - std::unique_ptr RooAbsAnaConvPdf::compileForNormSet(RooArgSet const &normSet, RooFit::Detail::CompileContext &ctx) const { diff --git a/roofit/roofitcore/src/RooAbsArg.cxx b/roofit/roofitcore/src/RooAbsArg.cxx index bdf6957c6e899..4f790ee178d7f 100644 --- a/roofit/roofitcore/src/RooAbsArg.cxx +++ b/roofit/roofitcore/src/RooAbsArg.cxx @@ -1629,102 +1629,6 @@ void RooAbsArg::optimizeCacheMode(const RooArgSet &observables, RooArgSet &optim } } -/// Find branch nodes with all-constant parameters, and add them to the list of -/// nodes that can be cached with a dataset in a test statistic calculation - -bool RooAbsArg::findConstantNodes(const RooArgSet &observables, RooArgSet &cacheList) -{ - RooLinkedList proc; - bool ret = findConstantNodes(observables, cacheList, proc); - - // If node can be optimized and hasn't been identified yet, add it to the list - coutI(Optimization) << "RooAbsArg::findConstantNodes(" << GetName() << "): components " << cacheList - << " depend exclusively on constant parameters and will be precalculated and cached" - << std::endl; - - return ret; -} - -/// Find branch nodes with all-constant parameters, and add them to the list of -/// nodes that can be cached with a dataset in a test statistic calculation - -bool RooAbsArg::findConstantNodes(const RooArgSet &observables, RooArgSet &cacheList, RooLinkedList &processedNodes) -{ - // Caching only applies to branch nodes - if (!isDerived()) { - return false; - } - - // Terminate call if this node was already processed (tree structure may be cyclical) - if (processedNodes.findArg(this)) { - return false; - } else { - processedNodes.Add(this); - } - - // Check if node depends on any non-constant parameter - bool canOpt(true); - RooArgSet paramSet; - getParameters(&observables, paramSet); - for (RooAbsArg *param : paramSet) { - if (!param->isConstant()) { - canOpt = false; - break; - } - } - - if (getAttribute("NeverConstant")) { - canOpt = false; - } - - if (canOpt) { - setAttribute("ConstantExpression"); - } - - // If yes, list node eligible for caching, if not test nodes one level down - if (canOpt || getAttribute("CacheAndTrack")) { - - if (!cacheList.find(*this) && dependsOnValue(observables) && !observables.find(*this)) { - - // Add to cache list - cxcoutD(Optimization) << "RooAbsArg::findConstantNodes(" << GetName() - << ") adding self to list of constant nodes" << std::endl; - - if (canOpt) - setAttribute("ConstantExpressionCached"); - cacheList.add(*this, false); - } - } - - if (!canOpt) { - - // If not, see if next level down can be cached - for (const auto server : _serverList) { - if (server->isDerived()) { - server->findConstantNodes(observables, cacheList, processedNodes); - } - } - } - - // Forward call to all cached contained in current object - for (Int_t i = 0; i < numCaches(); i++) { - getCache(i)->findConstantNodes(observables, cacheList, processedNodes); - } - - 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/RooAbsCacheElement.cxx b/roofit/roofitcore/src/RooAbsCacheElement.cxx index 4b21019e462ec..5b12c4fddc283 100644 --- a/roofit/roofitcore/src/RooAbsCacheElement.cxx +++ b/roofit/roofitcore/src/RooAbsCacheElement.cxx @@ -62,16 +62,3 @@ void RooAbsCacheElement::optimizeCacheMode(const RooArgSet& obs, RooArgSet& optN } } - - -//////////////////////////////////////////////////////////////////////////////// -/// Interface for constant term optimization calls. The default implementation is to forward all these -/// calls to all contained RooAbsArg objects as publicized through containedArg() - -void RooAbsCacheElement::findConstantNodes(const RooArgSet& obs, RooArgSet& cacheList, RooLinkedList& processedNodes) -{ - RooArgList list = containedArgs(FindConstantNodes) ; - for (const auto arg : list) { - arg->findConstantNodes(obs, cacheList, processedNodes); - } -} diff --git a/roofit/roofitcore/src/RooAbsCategory.cxx b/roofit/roofitcore/src/RooAbsCategory.cxx index 1a704086b94db..0d93b9a2e3c6c 100644 --- a/roofit/roofitcore/src/RooAbsCategory.cxx +++ b/roofit/roofitcore/src/RooAbsCategory.cxx @@ -527,18 +527,6 @@ void RooAbsCategory::fillTreeBranch(TTree& t) -//////////////////////////////////////////////////////////////////////////////// -/// (De)activate associate tree branch - -void RooAbsCategory::setTreeBranchStatus(TTree& t, bool active) -{ - TBranch* branch = t.GetBranch(Form("%s_idx",GetName())) ; - if (branch) { - t.SetBranchStatus(Form("%s_idx",GetName()),active?true:false) ; - } -} - - //////////////////////////////////////////////////////////////////////////////// /// Explicitly synchronize RooAbsCategory internal cache diff --git a/roofit/roofitcore/src/RooAbsData.cxx b/roofit/roofitcore/src/RooAbsData.cxx index b42af0276979a..13b7320efbfaf 100644 --- a/roofit/roofitcore/src/RooAbsData.cxx +++ b/roofit/roofitcore/src/RooAbsData.cxx @@ -182,7 +182,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)) { @@ -241,8 +240,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); } @@ -333,38 +331,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 @@ -2234,85 +2200,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) @@ -2383,13 +2270,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/RooAbsDataStore.cxx b/roofit/roofitcore/src/RooAbsDataStore.cxx index 67db7371eae33..a16309cdf457a 100644 --- a/roofit/roofitcore/src/RooAbsDataStore.cxx +++ b/roofit/roofitcore/src/RooAbsDataStore.cxx @@ -50,8 +50,4 @@ void RooAbsDataStore::printMultiline(std::ostream& os, Int_t /*content*/, bool v os << indent << " Observables: " << std::endl ; _vars.printStream(os,kName|kValue|kExtras|kTitle,kVerbose,indent+" ") ; } - - if(verbose && !_cachedVars.empty()) { - os << indent << " Caches " << _cachedVars << std::endl ; - } } 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 8d21d9afe7bc6..3870ec8fca329 100644 --- a/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx +++ b/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx @@ -113,7 +113,6 @@ RooAbsOptTestStatistic::RooAbsOptTestStatistic(const RooAbsOptTestStatistic &oth : RooAbsTestStatistic(other, name), _sealed(other._sealed), _sealNotice(other._sealNotice), - _skipZeroWeights(other._skipZeroWeights), _integrateBinsPrecision(other._integrateBinsPrecision) { // Don't do a thing in master mode @@ -408,86 +407,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; - } - - 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 ; - } - -} - - - //////////////////////////////////////////////////////////////////////////////// /// 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 @@ -508,133 +427,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 ; - } } @@ -687,11 +479,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()) ; @@ -766,10 +553,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..7c20885dbeee9 100644 --- a/roofit/roofitcore/src/RooAbsOptTestStatistic.h +++ b/roofit/roofitcore/src/RooAbsOptTestStatistic.h @@ -62,15 +62,9 @@ class RooAbsOptTestStatistic : public RooAbsTestStatistic { friend class RooAbsReal ; 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 @@ -81,12 +75,9 @@ class RooAbsOptTestStatistic : public RooAbsTestStatistic { bool _sealed = false; ///< Is test statistic sealed -- i.e. no access to data TString _sealNotice ; ///< User-defined notice shown when reading a sealed likelihood RooArgSet* _funcObsSet = nullptr; ///< List of observables in the pdf expression - RooArgSet _cachedNodes ; ///_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/RooAddPdf.cxx b/roofit/roofitcore/src/RooAddPdf.cxx index e45115d14cfb9..4b69d3373369f 100644 --- a/roofit/roofitcore/src/RooAddPdf.cxx +++ b/roofit/roofitcore/src/RooAddPdf.cxx @@ -934,16 +934,6 @@ bool RooAddPdf::isBinnedDistribution(const RooArgSet& obs) const } -//////////////////////////////////////////////////////////////////////////////// -/// Label OK'ed components of a RooAddPdf with cache-and-track - -void RooAddPdf::setCacheAndTrackHints(RooArgSet& trackNodes) -{ - RooRealSumPdf::setCacheAndTrackHints(_pdfList, trackNodes); -} - - - //////////////////////////////////////////////////////////////////////////////// /// Customized printing of arguments of a RooAddPdf to more intuitively reflect the contents of the /// product operator construction diff --git a/roofit/roofitcore/src/RooCompositeDataStore.cxx b/roofit/roofitcore/src/RooCompositeDataStore.cxx index 1d9f2d67c9af3..5c1572d973c9f 100644 --- a/roofit/roofitcore/src/RooCompositeDataStore.cxx +++ b/roofit/roofitcore/src/RooCompositeDataStore.cxx @@ -154,40 +154,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 @@ -390,57 +356,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 f3984892beccd..250522732c600 100644 --- a/roofit/roofitcore/src/RooDataSet.cxx +++ b/roofit/roofitcore/src/RooDataSet.cxx @@ -620,7 +620,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, ","); @@ -633,7 +632,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/RooMinimizer.cxx b/roofit/roofitcore/src/RooMinimizer.cxx index 2b57ce4e0a748..6a62be7ef4725 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" 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/RooNLLVar.cxx b/roofit/roofitcore/src/RooNLLVar.cxx index 6967afd8a5852..77247fab180fb 100644 --- a/roofit/roofitcore/src/RooNLLVar.cxx +++ b/roofit/roofitcore/src/RooNLLVar.cxx @@ -98,10 +98,6 @@ RooNLLVar::RooNLLVar(const char *name, const char *title, RooAbsPdf &pdf, RooAbs ++biter ; } } - - _skipZeroWeights = false; - } else { - _skipZeroWeights = true; } } diff --git a/roofit/roofitcore/src/RooObjCacheManager.cxx b/roofit/roofitcore/src/RooObjCacheManager.cxx index 12aeb776162db..cb5748950abb3 100644 --- a/roofit/roofitcore/src/RooObjCacheManager.cxx +++ b/roofit/roofitcore/src/RooObjCacheManager.cxx @@ -200,22 +200,3 @@ void RooObjCacheManager::printCompactTreeHook(std::ostream& os, const char *inde } - -//////////////////////////////////////////////////////////////////////////////// -/// If clearOnRedirect is false, forward constant term optimization calls to -/// cache elements - -void RooObjCacheManager::findConstantNodes(const RooArgSet& obs, RooArgSet& cacheList, RooLinkedList& processedNodes) -{ - if (!_allowOptimize) { - return ; - } - - for (Int_t i=0 ; ifindConstantNodes(obs,cacheList, processedNodes) ; - } - } -} - - diff --git a/roofit/roofitcore/src/RooProdPdf.cxx b/roofit/roofitcore/src/RooProdPdf.cxx index 88aa96fbc8735..08ebd2939cc45 100644 --- a/roofit/roofitcore/src/RooProdPdf.cxx +++ b/roofit/roofitcore/src/RooProdPdf.cxx @@ -1893,36 +1893,6 @@ std::list* RooProdPdf::binBoundaries(RooAbsRealLValue& obs, double xlo, } -//////////////////////////////////////////////////////////////////////////////// -/// Label OK'ed components of a RooProdPdf with cache-and-track, _and_ label all RooProdPdf -/// descendants with extra information about (conditional) normalization, needed to be able -/// to Cache-And-Track them outside the RooProdPdf context. - -void RooProdPdf::setCacheAndTrackHints(RooArgSet& trackNodes) -{ - for (const auto parg : _pdfList) { - - if (parg->canNodeBeCached()==Always) { - trackNodes.add(*parg) ; - - // Additional processing to fix normalization sets in case product defines conditional observables - if (RooArgSet* pdf_nset = findPdfNSet(static_cast(*parg))) { - // Check if conditional normalization is specified - using RooHelpers::getColonSeparatedNameString; - if (string("nset")==pdf_nset->GetName() && !pdf_nset->empty()) { - parg->setStringAttribute("CATNormSet",getColonSeparatedNameString(*pdf_nset).c_str()) ; - } - if (string("cset")==pdf_nset->GetName()) { - parg->setStringAttribute("CATCondSet",getColonSeparatedNameString(*pdf_nset).c_str()) ; - } - } else { - coutW(Optimization) << "RooProdPdf::setCacheAndTrackHints(" << GetName() << ") WARNING product pdf does not specify a normalization set for component " << parg->GetName() << std::endl ; - } - } - } -} - - //////////////////////////////////////////////////////////////////////////////// /// Customized printing of arguments of a RooProdPdf to more intuitively reflect the contents of the diff --git a/roofit/roofitcore/src/RooProduct.cxx b/roofit/roofitcore/src/RooProduct.cxx index de10d2bd4acc4..e44623b3e0ada 100644 --- a/roofit/roofitcore/src/RooProduct.cxx +++ b/roofit/roofitcore/src/RooProduct.cxx @@ -467,21 +467,6 @@ RooArgList RooProduct::CacheElem::containedArgs(Action) -//////////////////////////////////////////////////////////////////////////////// -/// Label OK'ed components of a RooProduct with cache-and-track - -void RooProduct::setCacheAndTrackHints(RooArgSet& trackNodes) -{ - RooArgSet comp(components()) ; - for (const auto parg : comp) { - if (parg->isDerived()) { - if (parg->canNodeBeCached()==Always) { - trackNodes.add(*parg) ; - } - } - } -} - //////////////////////////////////////////////////////////////////////////////// /// Customized printing of arguments of a RooProduct to more intuitively reflect the contents of the /// product operator construction diff --git a/roofit/roofitcore/src/RooRealIntegral.cxx b/roofit/roofitcore/src/RooRealIntegral.cxx index b4d901facf4bd..cbaf12bbb28bd 100644 --- a/roofit/roofitcore/src/RooRealIntegral.cxx +++ b/roofit/roofitcore/src/RooRealIntegral.cxx @@ -845,32 +845,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); - ChangeOperModeRAII operModeRAII; for (auto *arg : serverList) { arg->syncCache(); - if (arg->operMode() == RooAbsArg::AClean) { - operModeRAII.change(arg, RooAbsArg::Auto); - } } // Save current integral dependent values @@ -880,9 +860,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 40ad835591ff1..3289c709f2eb2 100644 --- a/roofit/roofitcore/src/RooRealMPFE.cxx +++ b/roofit/roofitcore/src/RooRealMPFE.cxx @@ -347,17 +347,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 ; @@ -666,33 +655,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..2727ddfecc8eb 100644 --- a/roofit/roofitcore/src/RooRealMPFE.h +++ b/roofit/roofitcore/src/RooRealMPFE.h @@ -55,14 +55,13 @@ 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 } ; State _state ; enum Message { SendReal=0, SendCat, Calculate, Retrieve, ReturnValue, Terminate, - ConstOpt, Verbose, LogEvalError, ApplyNLLW2, EnableOffset, CalculateNoOffset } ; + Verbose, LogEvalError, ApplyNLLW2, EnableOffset, CalculateNoOffset } ; void initialize() ; void initVars() ; diff --git a/roofit/roofitcore/src/RooRealSumFunc.cxx b/roofit/roofitcore/src/RooRealSumFunc.cxx index 29b75c2974c0f..37993f182d02c 100644 --- a/roofit/roofitcore/src/RooRealSumFunc.cxx +++ b/roofit/roofitcore/src/RooRealSumFunc.cxx @@ -143,12 +143,6 @@ std::list *RooRealSumFunc::plotSamplingHint(RooAbsRealLValue &obs, doubl return RooRealSumPdf::plotSamplingHint(_funcList, obs, xlo, xhi); } -//_____________________________________________________________________________ -void RooRealSumFunc::setCacheAndTrackHints(RooArgSet &trackNodes) -{ - RooRealSumPdf::setCacheAndTrackHints(_funcList, trackNodes); -} - /// Customized printing of arguments of a RooRealSumFunc to more intuitively /// reflect the contents of the product operator construction. diff --git a/roofit/roofitcore/src/RooRealSumPdf.cxx b/roofit/roofitcore/src/RooRealSumPdf.cxx index 2be1264e85303..ade43ac638dbd 100644 --- a/roofit/roofitcore/src/RooRealSumPdf.cxx +++ b/roofit/roofitcore/src/RooRealSumPdf.cxx @@ -662,25 +662,6 @@ RooRealSumPdf::plotSamplingHint(RooArgList const &funcList, RooAbsRealLValue &ob -//////////////////////////////////////////////////////////////////////////////// -/// Label OK'ed components of a RooRealSumPdf with cache-and-track - -void RooRealSumPdf::setCacheAndTrackHints(RooArgSet& trackNodes) -{ - setCacheAndTrackHints(_funcList, trackNodes); -} - - -void RooRealSumPdf::setCacheAndTrackHints(RooArgList const& funcList, RooArgSet& trackNodes) -{ - for (const auto sarg : funcList) { - if (sarg->canNodeBeCached()==Always) { - trackNodes.add(*sarg) ; - } - } -} - - //////////////////////////////////////////////////////////////////////////////// /// Customized printing of arguments of a RooRealSumPdf to more intuitively reflect the contents of the /// product operator construction diff --git a/roofit/roofitcore/src/RooStringVar.cxx b/roofit/roofitcore/src/RooStringVar.cxx index 8e03795e4672d..57f46488181bc 100644 --- a/roofit/roofitcore/src/RooStringVar.cxx +++ b/roofit/roofitcore/src/RooStringVar.cxx @@ -131,15 +131,3 @@ void RooStringVar::fillTreeBranch(TTree& t) -//////////////////////////////////////////////////////////////////////////////// -/// (De)Activate associated tree branch - -void RooStringVar::setTreeBranchStatus(TTree& t, bool active) -{ - TBranch* branch = t.GetBranch(GetName()) ; - if (branch) { - t.SetBranchStatus(GetName(),active?true:false) ; - } -} - - 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 c72e0a8beae32..d51d5e97d5185 100644 --- a/roofit/roofitcore/src/RooVectorDataStore.cxx +++ b/roofit/roofitcore/src/RooVectorDataStore.cxx @@ -272,11 +272,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); } @@ -302,8 +297,6 @@ RooVectorDataStore::~RooVectorDataStore() for (auto elm : _catStoreList) { delete elm; } - - delete _cache ; } @@ -363,10 +356,6 @@ const RooArgSet* RooVectorDataStore::get(Int_t index) const // Update current weight cache _currentWeightIndex = index; - if (_cache) { - _cache->get(index) ; - } - return &_vars; } @@ -741,280 +730,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 - - 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 ; - std::vector> ownedNsets; - - // 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) { - RooArgSet anset = RooHelpers::selectFromArgSet(nset ? *nset : RooArgSet{}, catNset); - ownedNsets.emplace_back(anset.selectCommon(*argObs)); - normSet = ownedNsets.back().get(); - } - const char* catCset = arg->getStringAttribute("CATCondSet") ; - if (catCset) { - 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 - 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) @@ -1139,15 +854,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/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/ConstantTermsOptimizer.h b/roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.h deleted file mode 100644 index b86a2b99fc3a3..0000000000000 --- a/roofit/roofitcore/src/TestStatistics/ConstantTermsOptimizer.h +++ /dev/null @@ -1,38 +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) - */ - -#ifndef ROOT_ROOFIT_TESTSTATISTICS_ConstantTermsOptimizer -#define ROOT_ROOFIT_TESTSTATISTICS_ConstantTermsOptimizer - -// forward declarations -class RooAbsReal; -class RooArgSet; -class RooAbsData; - -namespace RooFit { -namespace TestStatistics { - -// this is a class only for convenience: it saves multiple friend definitions in RooAbsData for otherwise free functions -struct ConstantTermsOptimizer { - static void enableConstantTermsOptimization(RooAbsReal *function, RooArgSet *norm_set, RooAbsData *dataset, - bool applyTrackingOpt); - static void - optimizeCaching(RooAbsReal *function, RooArgSet *norm_set, RooAbsData *dataset, RooArgSet *observables = nullptr); - static void disableConstantTermsOptimization(RooAbsReal *function, RooArgSet *norm_set, RooAbsData *dataset, - RooArgSet *observables = nullptr); - static RooArgSet requiredExtraObservables(); -}; - -} // namespace TestStatistics -} // namespace RooFit - -#endif // ROOT_ROOFIT_TESTSTATISTICS_ConstantTermsOptimizer diff --git a/roofit/roofitcore/src/TestStatistics/LikelihoodJob.cxx b/roofit/roofitcore/src/TestStatistics/LikelihoodJob.cxx index 1918a837e5905..6efe297299934 100644 --- a/roofit/roofitcore/src/TestStatistics/LikelihoodJob.cxx +++ b/roofit/roofitcore/src/TestStatistics/LikelihoodJob.cxx @@ -44,10 +44,6 @@ LikelihoodJob::LikelihoodJob(std::shared_ptr likelihood, } // This is a separate function (instead of just in ctor) for historical reasons. -// Its predecessor RooRealMPFE::initVars() was used from multiple ctors, but also -// from RooRealMPFE::constOptimizeTestStatistic at the end, which makes sense, -// because it might change the set of variables. We may at some point want to do -// this here as well. void LikelihoodJob::init_vars() { // Empty current lists diff --git a/roofit/roofitcore/src/TestStatistics/LikelihoodSerial.cxx b/roofit/roofitcore/src/TestStatistics/LikelihoodSerial.cxx index c85bcf369ae34..ed75430da4935 100644 --- a/roofit/roofitcore/src/TestStatistics/LikelihoodSerial.cxx +++ b/roofit/roofitcore/src/TestStatistics/LikelihoodSerial.cxx @@ -42,10 +42,6 @@ LikelihoodSerial::LikelihoodSerial(std::shared_ptr likelihood, /// \brief Helper function for the constructor. /// /// This is a separate function (instead of just in ctor) for historical reasons. -/// Its predecessor RooRealMPFE::initVars() was used from multiple ctors, but also -/// from RooRealMPFE::constOptimizeTestStatistic at the end, which makes sense, -/// because it might change the set of variables. We may at some point want to do -/// this here as well. void LikelihoodSerial::initVars() { // Empty current lists 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 badd019b14964..05d124009b839 100644 --- a/roofit/roofitcore/src/TestStatistics/RooAbsL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooAbsL.cxx @@ -11,9 +11,7 @@ */ #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 +228,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 +235,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 c054b702ddec0..7e8fd790e0522 100644 --- a/roofit/roofitcore/src/TestStatistics/RooUnbinnedL.cxx +++ b/roofit/roofitcore/src/TestStatistics/RooUnbinnedL.cxx @@ -97,27 +97,6 @@ bool RooUnbinnedL::setApplyWeightSquared(bool flag) return false; } -////////////////////////////////////////////////////////////////////////////////// - -/// With the vectorizing evaluation backends, the pdf is compiled for a fixed -/// normalization set and evaluated outside of the RooFit data store, so the -/// constant term optimization is not applicable. Attempting it anyway is not -/// only useless: caching constant branches means snapshotting parts of the -/// compiled computation graph into the data store, which classes like -/// RooFit::Detail::RooNormalizedPdf don't support. Refuse the request instead. -void RooUnbinnedL::constOptimizeTestStatistic(RooAbsArg::ConstOpCode opcode, bool doAlsoTrackingOpt) -{ - if (evaluator_) { - oocoutW((TObject *)nullptr, Optimization) - << "RooUnbinnedL::constOptimizeTestStatistic(" << GetName() - << ") the constant term optimization only applies to likelihoods evaluated with EvalBackend::Legacy(), " - "ignoring the request" - << std::endl; - return; - } - RooAbsL::constOptimizeTestStatistic(opcode, doAlsoTrackingOpt); -} - namespace { using ComputeResult = std::pair, double>; @@ -234,19 +213,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 { - // The cache-and-track optimization tracks staleness of the cached - // branches globally, but recalculateCache() only refreshes the requested - // event range. A cache that was refreshed for one event section hence - // reports itself as up-to-date for all other sections as well, even - // though their rows may still hold values from an older parameter point. - // This happens when event-range tasks migrate between workers in - // RooFit::MultiProcess likelihood splitting. Force a full update of the - // cached branches whenever the evaluated section changes. - if (!(events == lastCacheSection_)) { - data_->store()->forceCacheUpdate(); - lastCacheSection_ = events; - } - 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 5335a8f322ff8..da077eb6de785 100644 --- a/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx +++ b/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx @@ -475,7 +475,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 c32d97fecf680..d12c7d6c08438 100644 --- a/roofit/roofitcore/test/testRooProdPdf.cxx +++ b/roofit/roofitcore/test/testRooProdPdf.cxx @@ -23,57 +23,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 1c9ac6e8d8c60..b9fbddcb864f3 100644 --- a/roofit/roofitcore/test/testTestStatistics.cxx +++ b/roofit/roofitcore/test/testTestStatistics.cxx @@ -158,12 +158,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())), @@ -209,13 +209,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())), diff --git a/roofit/roostats/inc/RooStats/MaxLikelihoodEstimateTestStat.h b/roofit/roostats/inc/RooStats/MaxLikelihoodEstimateTestStat.h index 4fcf0331f0813..ef10deb1cd372 100644 --- a/roofit/roostats/inc/RooStats/MaxLikelihoodEstimateTestStat.h +++ b/roofit/roostats/inc/RooStats/MaxLikelihoodEstimateTestStat.h @@ -88,7 +88,6 @@ class MaxLikelihoodEstimateTestStat : public TestStatistic { //LM: RooMinimizer.setPrintLevel has +1 offset - so subtract here -1 minim.setPrintLevel(fPrintLevel-1); int status = -1; - // minim.optimizeConst(true); for (int tries = 0, maxtries = 4; tries <= maxtries; ++tries) { // status = minim.minimize(fMinimizer, ROOT::Math::MinimizerOptions::DefaultMinimizerAlgo().c_str()); status = minim.minimize(fMinimizer, "Minimize"); diff --git a/roofit/roostats/src/AsymptoticCalculator.cxx b/roofit/roostats/src/AsymptoticCalculator.cxx index a54ccd6771876..ce9d3fc7df743 100644 --- a/roofit/roostats/src/AsymptoticCalculator.cxx +++ b/roofit/roostats/src/AsymptoticCalculator.cxx @@ -406,7 +406,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(); @@ -464,8 +463,6 @@ double EvaluateNLL(RooStats::ModelConfig const &modelConfig, RooAbsData &data, c if (fitResult) *fitResult = std::move(result); - - minim.optimizeConst(false); } double muTest = 0; diff --git a/roofit/roostats/src/ProfileLikelihoodCalculator.cxx b/roofit/roostats/src/ProfileLikelihoodCalculator.cxx index 8ceced2cb0c41..e959d35e068b2 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 1c3009c4822f1..38a0bfe00319d 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 0707cb107c4e7..ddde2f9beb4af 100644 --- a/roofit/xroofit/src/xRooFit.cxx +++ b/roofit/xroofit/src/xRooFit.cxx @@ -482,8 +482,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; } @@ -514,8 +512,6 @@ std::shared_ptr xRooFit::defaultFitConfig() fitConfig.MinimizerOptions().SetExtraOptions(ROOT::Math::GenAlgoOptions()); // have to const cast to set extra options auto extraOpts = const_cast(fitConfig.MinimizerOptions().ExtraOptions()); - extraOpts->SetValue("OptimizeConst", 2); // if 0 will disable constant term optimization and cache-and-track of the - // NLL. 1 = just caching, 2 = cache and track #if ROOT_VERSION_CODE >= ROOT_VERSION(6, 29, 00) extraOpts->SetValue( "StrategySequence", @@ -623,10 +619,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 { @@ -1021,26 +1013,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(); @@ -1331,10 +1303,6 @@ std::shared_ptr xRooFit::minimize(RooAbsReal &nll, } } - // DO NOT DO THIS - seems to mess with the NLL function in a way that breaks the cache - reactivating wont fix - // if(constOptimize) { _minimizer->optimizeConst(0); } // doing this because saw happens in RooAbsPdf::minimizeNLL - // method - // signal(SIGINT,gOldHandlerr); out = std::unique_ptr{_minimizer->save(fitName, resultTitle)}; diff --git a/roofit/xroofit/src/xRooNLLVar.cxx b/roofit/xroofit/src/xRooNLLVar.cxx index d1bf059351b38..4fcb5f1228e9f 100644 --- a/roofit/xroofit/src/xRooNLLVar.cxx +++ b/roofit/xroofit/src/xRooNLLVar.cxx @@ -1268,17 +1268,10 @@ std::shared_ptr xRooNLLVar::func() const if (!(*this)) { const_cast(this)->reinitialize(); } else if (auto f = std::unique_ptr(fConstVars->selectByAttrib("Constant", false)); !f->empty()) { - // have to reinitialize if const par values have changed - const optimization forces this + // have to reinitialize if const par values have changed // TODO: currently changes to globs also triggers this since the vars includes globs (vars are the non-obs pars) // std::cout << "Reinitializing because of change of const parameters:" << f->contentsString() << std::endl; const_cast(this)->reinitialize(); - - // note ... it may be sufficient here to do: - // nll.constOptimizeTestStatistic(RooAbsArg::ConfigChange, constOptimize>1 /* do tracking too if >1 */); // - // trigger a re-evaluate of which nodes to cache-and-track nll.constOptimizeTestStatistic(RooAbsArg::ValueChange, - // constOptimize>1); // update the cache values -- is this needed?? - // this forces the optimization to be redone - // for now leave as a reinitialize though, until had a chance to test this properly } if (fGlobs && fFuncGlobs) { *fFuncGlobs = *fGlobs; @@ -1307,11 +1300,6 @@ void xRooNLLVar::SetOption(const RooCmdArg &opt) } else if (strcmp(opt.GetName(), "PrintLevel") == 0) { fitConfig()->MinimizerOptions().SetPrintLevel(opt.getInt(0)); } else { - if (strcmp(opt.GetName(), "Optimize") == 0) { - // this flag will trigger constOptimizeTestStatistic to be called on the nll in createNLL method - // we should ensure that the fitconfig setting is consistent with it ... - fitConfigOptions()->SetValue("OptimizeConst", opt.getInt(0)); - } if (auto prevObject = fOpts->FindObject(opt.GetName()); prevObject) { // replace previous option fOpts->Replace(prevObject, opt.Clone(nullptr)); diff --git a/tutorials/roofit/roofit/rf712_lagrangianmorphfit.py b/tutorials/roofit/roofit/rf712_lagrangianmorphfit.py index dc70198d27a8b..097f49f862e86 100644 --- a/tutorials/roofit/roofit/rf712_lagrangianmorphfit.py +++ b/tutorials/roofit/roofit/rf712_lagrangianmorphfit.py @@ -89,7 +89,7 @@ # wrapper pdf to normalise morphing function to a morphing pdf model = ROOT.RooWrapperPdf("wrap_pdf", "wrap_pdf", morphfunc) -fitres = model.fitTo(pseudo_dh, SumW2Error=True, Optimize=False, Save=True, PrintLevel=-1) +fitres = model.fitTo(pseudo_dh, SumW2Error=True, Save=True, PrintLevel=-1) # run the fit # Get the correlation matrix hcorr = fitres.correlationHist()