diff --git a/README/ReleaseNotes/v642/index.md b/README/ReleaseNotes/v642/index.md index 8eb3d7de6b291..bcfa03a21d92f 100644 --- a/README/ReleaseNotes/v642/index.md +++ b/README/ReleaseNotes/v642/index.md @@ -45,6 +45,7 @@ The following people have contributed to this new version: Note that `all=ON` enables several of these options, so building with `-Dall=ON` now requires all of their dependencies to be installed, or the unwanted ones to be disabled explicitly. Build options that are enabled by default, such as `pyroot`, `opengl`, `xml`, `sqlite`, `davix`, `curl`, `tmva-cpu` or `tpython` are not affected: they are still disabled automatically when their dependencies are missing. * The option `fail-on-missing=OFF` will no longer be honored for CMake ROOT build options that have easy-to-install dependencies (e.g. via homebrew or apt-get), such as those required by options `cfitsio`, `civetweb`, `fftw3`, `imt`, `mathmore`, `nlohmann_json`, `tmva-cpu`, `unuran`, `vdt` or `xrootd`. Before, associated `builtin_option` was automatically turned ON (or the opt-in feature turned to OFF), now, user has to install system package or manually set `builtin_option` to `ON` or opt-in feature to `OFF`. +* The legacy evaluation backend of RooFit and the related `RooFit::BatchMode()` command argument are deprecated and will be removed in ROOT 6.44. See the RooFit section below for details. * The method `RooRealVar::removeRange()` and the corresponding method in `RooErrorVar` that were deprecated in ROOT 6.40 are now removed. * The overloads of `RooAbsReal::createChi2()` and `RooAbsReal::chi2FitTo()` that take unbinned **RooDataSet** data objects were deprecated in ROOT 6.40 and are now removed. * The **RooStats::HybridPlot** class and the related **HybridResult::GetPlot** method were deprecated in ROOT 6.40 and are now removed. @@ -144,6 +145,26 @@ 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 +### Deprecation of the legacy evaluation backend + +The `legacy` evaluation backend for likelihood and chi-square fits is deprecated and will be removed in ROOT 6.44. +It was superseded by the vectorized `cpu` backend, which is the default since ROOT 6.32. +After the removal of the constant term optimization (see below), the legacy backend also has no performance-relevant feature left that would justify its continued maintenance. + +Selecting the legacy backend with `RooFit::EvalBackend("legacy")` now prints a deprecation warning whenever a likelihood or chi-square object is created with it, and the `RooFit::EvalBackend::Legacy()` factory function is marked as deprecated, resulting in compiler warnings. + +The **RooFit::BatchMode()** command argument, which was superseded by `RooFit::EvalBackend()` in ROOT 6.28, is deprecated at the same time and will also be removed in ROOT 6.44. +Note that the C++ declarations of `RooFit::BatchMode()` had been unintentionally absent since ROOT 6.30; they are restored in this release, marked as deprecated, to give downstream code a proper migration window. + +The removal in ROOT 6.44 will also include: + + * the implementation classes of the legacy test statistics: **RooNLLVar**, **RooChi2Var**, **RooAbsOptTestStatistic** and **RooAbsTestStatistic** (their headers are not part of the public interface anymore since ROOT 6.32, but they are still installed), + * the old multiprocessing mechanism of the legacy backend, consisting of the **RooRealMPFE** class and the underlying **BidirMMapPipe**, + * the `nll::name[pdf,data]` and `chi2::name[pdf,data]` expressions in the `RooWorkspace::factory()` language, which instantiate the removed classes directly. + +Users are strongly encouraged to switch to the default `cpu` evaluation backend, i.e., to simply not pass any `EvalBackend()` or `BatchMode()` command argument. +If the default backend does not work for a given use case, **please report it by opening an issue on the ROOT GitHub repository**. + ### Removal of the constant term optimization for legacy test statistic classes The **RooFit::Optimize()** option (constant term optimization) was deprecated in ROOT 6.40, and its functionality is now removed. diff --git a/roofit/histfactory/test/testHistFactory.cxx b/roofit/histfactory/test/testHistFactory.cxx index 594d7b73395b0..29acd2c4ded64 100644 --- a/roofit/histfactory/test/testHistFactory.cxx +++ b/roofit/histfactory/test/testHistFactory.cxx @@ -444,7 +444,7 @@ TEST_P(HFFixtureEval, Evaluation) const double systEps = 1e-6; const MakeModelMode makeModelMode = std::get<0>(GetParam()); - const bool useBatchMode = std::get<2>(GetParam()) != RooFit::EvalBackend::Legacy(); + const bool useBatchMode = std::get<2>(GetParam()) != RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy); RooHelpers::HijackMessageStream evalMessages(RooFit::INFO, RooFit::FastEvaluations); diff --git a/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx b/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx index 1a9ccfac3d162..26c4544ba8856 100644 --- a/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx +++ b/roofit/roofit/test/vectorisedPDFs/VectorisedPDFTests.cxx @@ -522,7 +522,7 @@ std::unique_ptr PDFTest::runScalarFit(RooAbsPdf *pdf) } MyTimer singleTimer("Fitting scalar mode " + _name); - std::unique_ptr result{pdf->fitTo(*_dataFit, RooFit::EvalBackend::Legacy(), RooFit::SumW2Error(false), + std::unique_ptr result{pdf->fitTo(*_dataFit, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy), RooFit::SumW2Error(false), RooFit::PrintLevel(_printLevel), RooFit::Save(), _multiProcess > 0 ? RooFit::NumCPU(_multiProcess) : RooCmdArg())}; std::cout << singleTimer; diff --git a/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h b/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h index 18b1450cf82ce..4888f883aecd0 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/RooUnbinnedL.h @@ -33,7 +33,7 @@ namespace TestStatistics { class RooUnbinnedL : public RooAbsL { public: RooUnbinnedL(RooAbsPdf *pdf, RooAbsData *data, RooAbsL::Extended extended = RooAbsL::Extended::Auto, - RooFit::EvalBackend evalBackend = RooFit::EvalBackend::Legacy()); + RooFit::EvalBackend evalBackend = RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)); RooUnbinnedL(const RooUnbinnedL &other); ~RooUnbinnedL() override; bool setApplyWeightSquared(bool flag); diff --git a/roofit/roofitcore/inc/RooFit/TestStatistics/buildLikelihood.h b/roofit/roofitcore/inc/RooFit/TestStatistics/buildLikelihood.h index 19d6aa4d93f63..77eea6291bb16 100644 --- a/roofit/roofitcore/inc/RooFit/TestStatistics/buildLikelihood.h +++ b/roofit/roofitcore/inc/RooFit/TestStatistics/buildLikelihood.h @@ -48,7 +48,7 @@ class NLLFactory { RooArgSet _externalConstraints; RooArgSet _globalObservables; std::string _globalObservablesTag; - RooFit::EvalBackend _evalBackend = RooFit::EvalBackend::Legacy(); + RooFit::EvalBackend _evalBackend = RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy); }; /// Delegating function to build a likelihood without additional arguments. diff --git a/roofit/roofitcore/inc/RooGlobalFunc.h b/roofit/roofitcore/inc/RooGlobalFunc.h index 2ad697c57b88a..180d7308c73c5 100644 --- a/roofit/roofitcore/inc/RooGlobalFunc.h +++ b/roofit/roofitcore/inc/RooGlobalFunc.h @@ -242,12 +242,15 @@ RooCmdArg Parallelize(int nWorkers) ; RooCmdArg ModularL(bool flag=false) ; RooCmdArg TimingAnalysis(bool timingAnalysis) ; -//RooCmdArg BatchMode(std::string const& batchMode="cpu"); -//// The const char * overload is necessary, otherwise the compiler will cast a -//// C-Style string to a bool and choose the BatchMode(bool) overload if one -//// calls for example BatchMode("off"). -//inline RooCmdArg BatchMode(const char * batchMode) { return BatchMode(std::string(batchMode)); } -//inline RooCmdArg BatchMode(bool batchModeOn) { return BatchMode(batchModeOn ? "cpu" : "off"); } +RooCmdArg BatchMode(std::string const &batchMode = "cpu") + R__DEPRECATED(6, 44, "Use EvalBackend() instead of BatchMode()."); +// The const char * overload is necessary, otherwise the compiler will cast a +// C-Style string to a bool and choose the BatchMode(bool) overload if one +// calls for example BatchMode("off"). +RooCmdArg BatchMode(const char *batchMode) + R__DEPRECATED(6, 44, "Use EvalBackend() instead of BatchMode()."); +RooCmdArg BatchMode(bool batchModeOn) + R__DEPRECATED(6, 44, "Use EvalBackend() instead of BatchMode()."); RooCmdArg IntegrateBins(double precision); @@ -263,7 +266,10 @@ class EvalBackend : public RooCmdArg { EvalBackend(std::string const &name); - static EvalBackend Legacy(); + static EvalBackend Legacy() + R__DEPRECATED(6, 44, "The legacy evaluation backend will be removed in ROOT 6.44. " + "Use the default \"cpu\" evaluation backend, i.e. simply don't pass any EvalBackend() " + "command argument."); static EvalBackend Cpu(); static EvalBackend Cuda(); static EvalBackend Codegen(); diff --git a/roofit/roofitcore/src/FitHelpers.cxx b/roofit/roofitcore/src/FitHelpers.cxx index 7431401a962c2..e7f8a60e7c114 100644 --- a/roofit/roofitcore/src/FitHelpers.cxx +++ b/roofit/roofitcore/src/FitHelpers.cxx @@ -60,6 +60,19 @@ namespace { constexpr int extendedFitDefault = 2; +#ifdef ROOFIT_LEGACY_EVAL_BACKEND +/// Print a deprecation warning when the legacy evaluation backend is selected for a fit. +void printLegacyEvalBackendWarning(RooAbsReal const &topLevelArg) +{ + oocoutW(&topLevelArg, InputArguments) + << "The legacy evaluation backend is deprecated and will be removed in ROOT 6.44.\n" + "Please use the default \"cpu\" evaluation backend instead, i.e., don't pass RooFit::EvalBackend(\"legacy\")\n" + "or RooFit::BatchMode(\"off\") anymore. If the default backend does not work for your use case, please\n" + "report it by opening an issue on the ROOT GitHub repository." + << std::endl; +} +#endif + //////////////////////////////////////////////////////////////////////////////// /// Use the asymptotically correct approach to estimate errors in the presence of weights. /// This is slower but more accurate than `SumW2Error`. See also https://arxiv.org/abs/1911.01303). @@ -924,6 +937,8 @@ std::unique_ptr createNLL(RooAbsPdf &pdf, RooAbsData &data, const Ro std::unique_ptr nll; #ifdef ROOFIT_LEGACY_EVAL_BACKEND + printLegacyEvalBackendWarning(pdf); + bool verbose = pc.getInt("verbose"); int numcpu = pc.getInt("numcpu"); @@ -1140,6 +1155,8 @@ std::unique_ptr createChi2(RooAbsReal &real, RooDataHist &data, cons } #ifdef ROOFIT_LEGACY_EVAL_BACKEND + printLegacyEvalBackendWarning(real); + RooAbsTestStatistic::Configuration cfg; RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::CollectErrors); diff --git a/roofit/roofitcore/src/RooAbsPdf.cxx b/roofit/roofitcore/src/RooAbsPdf.cxx index 74e111b66e3ca..78c7ff8f83af9 100644 --- a/roofit/roofitcore/src/RooAbsPdf.cxx +++ b/roofit/roofitcore/src/RooAbsPdf.cxx @@ -856,7 +856,9 @@ double RooAbsPdf::extendedTerm(RooAbsData const& data, bool weightSquared, bool * This backend re-uses code from the **cpu** backend, but compiled in CUDA kernels. * Hence, the results are expected to be identical, modulo some numerical differences that can arise from the different order in which the GPU is summing the log probabilities. * This backend can drastically speed up the fit if all RooAbsArg object in the model support it. - * **legacy** The original likelihood evaluation method. + * **legacy** \deprecated The original likelihood evaluation method. + * It is deprecated and will be removed in ROOT 6.44, together with the deprecated BatchMode() command argument + * (where the legacy backend corresponded to `BatchMode("off")`). * Evaluates the PDF for each single data entry at a time before summing the negative log probabilities. * It supports multi-threading, but you might need more than 20 threads to maybe see about 10% performance gain over the default cpu-backend (that runs currently only on a single thread). * **codegen** **Experimental** - Generates and compiles minimal C++ code for the NLL on-the-fly and wraps it in the returned RooAbsReal. diff --git a/roofit/roofitcore/src/RooGlobalFunc.cxx b/roofit/roofitcore/src/RooGlobalFunc.cxx index 1ed27e1a2d82c..f6c183b208005 100644 --- a/roofit/roofitcore/src/RooGlobalFunc.cxx +++ b/roofit/roofitcore/src/RooGlobalFunc.cxx @@ -476,21 +476,41 @@ RooCmdArg TimingAnalysis(bool flag) { return RooCmdArg("TimingAnalysis", flag, 0, 0, 0, nullptr, nullptr, nullptr); } -RooCmdArg BatchMode(std::string const &batchMode) +namespace { + +RooCmdArg batchModeImpl(std::string const &batchMode) { oocoutW(nullptr, InputArguments) - << "The BatchMode() command argument is deprecated. Please use EvalBackend() instead." << std::endl; + << "The BatchMode() command argument is deprecated and will be removed in ROOT 6.44, together with the legacy\n" + "evaluation backend that corresponds to BatchMode(\"off\"). Please use EvalBackend() instead, or simply pass\n" + "no command argument to get the default \"cpu\" evaluation backend." + << std::endl; std::string lower = batchMode; std::transform(lower.begin(), lower.end(), lower.begin(), [](unsigned char c) { return std::tolower(c); }); if (lower == "off") { - return EvalBackend::Legacy(); + return EvalBackend(EvalBackend::Value::Legacy); } else if (lower == "cpu") { - return EvalBackend::Cpu(); + return EvalBackend(EvalBackend::Value::Cpu); } else if (lower == "cuda") { - return EvalBackend::Cuda(); + return EvalBackend(EvalBackend::Value::Cuda); } throw std::runtime_error("Only supported string values for BatchMode() are \"off\", \"cpu\", or \"cuda\"."); } + +} // namespace + +RooCmdArg BatchMode(std::string const &batchMode) +{ + return batchModeImpl(batchMode); +} +RooCmdArg BatchMode(const char *batchMode) +{ + return batchModeImpl(batchMode); +} +RooCmdArg BatchMode(bool batchModeOn) +{ + return batchModeImpl(batchModeOn ? "cpu" : "off"); +} /// Integrate the PDF over bins. Improves accuracy for binned fits. Switch off using `0.` as argument. \see /// RooAbsPdf::fitTo(). RooCmdArg IntegrateBins(double precision) diff --git a/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx b/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx index da077eb6de785..a2e43b78e7e65 100644 --- a/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx +++ b/roofit/roofitcore/test/TestStatistics/testLikelihoodGradientJob.cxx @@ -250,7 +250,7 @@ TEST_P(LikelihoodGradientJobTest, GaussianND) std::unique_ptr values; RooAbsPdf *pdf; std::unique_ptr data; - std::tie(nll, pdf, data, values) = generate_ND_gaussian_pdf_nll(w, N, 1000, RooFit::EvalBackend::Legacy()); + std::tie(nll, pdf, data, values) = generate_ND_gaussian_pdf_nll(w, N, 1000, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)); RooArgSet savedValues; values->snapshot(savedValues); @@ -632,7 +632,7 @@ TEST_P(LikelihoodGradientJobErrorTest, ErrorHandling) } else { data = std::unique_ptr{pdf->generate(*w.var("m"), 10000)}; } - std::unique_ptr nll{pdf->createNLL(*data, RooFit::EvalBackend::Legacy())}; + std::unique_ptr nll{pdf->createNLL(*data, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))}; // if m0 were constant (i.e. setConstant(true)), the fit would converge without errors, because m0 outside of the // physical area of the Argus distribution is what causes the errors in the line search phase of the fit @@ -693,7 +693,7 @@ TEST_P(LikelihoodGradientJobErrorTest, FitSimpleLinear) } else { data = std::unique_ptr{pdf.generate(x, 1000)}; } - std::unique_ptr nll(pdf.createNLL(*data, RooFit::EvalBackend::Legacy())); + std::unique_ptr nll(pdf.createNLL(*data, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))); RooArgSet normSet{x}; ASSERT_FALSE(std::isnan(pdf.getVal(normSet))); @@ -841,7 +841,7 @@ TEST_P(LikelihoodGradientJobBinnedErrorTest, TriggerMuLEZero) values->snapshot(savedValues); // legacy RooFit fit - std::unique_ptr nll(w.pdf("model")->createNLL(h_data, RooFit::EvalBackend::Legacy())); + std::unique_ptr nll(w.pdf("model")->createNLL(h_data, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))); double nll0BeforeFit = nll->getVal(); @@ -933,7 +933,7 @@ TEST(MinuitFcnGrad, DISABLED_CompareToRooMinimizerFcn) std::unique_ptr nll_vanilla{pdf->createNLL(*data, RooFit::Constrain(*nuisance_parameters), RooFit::GlobalObservables(*global_observables), - RooFit::EvalBackend::Legacy() + RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy) /*, RooFit::Offset(true)*/)}; double vanilla_val = nll_vanilla->getVal(); diff --git a/roofit/roofitcore/test/TestStatistics/testLikelihoodJob.cxx b/roofit/roofitcore/test/TestStatistics/testLikelihoodJob.cxx index 4eae3fc591b27..a5f2c6a1ee05a 100644 --- a/roofit/roofitcore/test/TestStatistics/testLikelihoodJob.cxx +++ b/roofit/roofitcore/test/TestStatistics/testLikelihoodJob.cxx @@ -182,7 +182,7 @@ TEST_F(LikelihoodJobTest, UnbinnedGaussianND) using namespace RooFit; unsigned int N = 4; - std::tie(nll, pdf, data, values) = generate_ND_gaussian_pdf_nll(w, N, 1000, EvalBackend::Legacy()); + std::tie(nll, pdf, data, values) = generate_ND_gaussian_pdf_nll(w, N, 1000, EvalBackend(EvalBackend::Value::Legacy)); likelihood = TestStatistics::buildLikelihood(pdf, data.get()); // dummy offsets (normally they are shared with other objects): SharedOffset offset; @@ -487,7 +487,7 @@ TEST_F(LikelihoodJobSimBinnedConstrainedTest, ConstrainedAndOffset) // RooAbsTestStatistic. nll = std::unique_ptr{pdf->createNLL(*data, Constrain(*w.var("alpha_bkg_A")), GlobalObservables(*w.var("alpha_bkg_obs_B")), Offset("initial"), - EvalBackend::Legacy())}; + EvalBackend(EvalBackend::Value::Legacy))}; // -------- @@ -567,7 +567,7 @@ TEST_P(LikelihoodJobSplitStrategies, DISABLED_SimBinnedConstrainedAndOffset) // RooAbsTestStatistic. nll = std::unique_ptr{pdf->createNLL(*data, Constrain(*w.var("alpha_bkg_A")), GlobalObservables(*w.var("alpha_bkg_obs_B")), Offset("initial"), - EvalBackend::Legacy())}; + EvalBackend(EvalBackend::Value::Legacy))}; // -------- diff --git a/roofit/roofitcore/test/TestStatistics/testLikelihoodSerial.cxx b/roofit/roofitcore/test/TestStatistics/testLikelihoodSerial.cxx index fe267ccbe54a3..8751767524352 100644 --- a/roofit/roofitcore/test/TestStatistics/testLikelihoodSerial.cxx +++ b/roofit/roofitcore/test/TestStatistics/testLikelihoodSerial.cxx @@ -104,7 +104,7 @@ TEST_F(LikelihoodSerialTest, UnbinnedGaussianND) { unsigned int N = 4; - std::tie(nll, pdf, data, values) = generate_ND_gaussian_pdf_nll(w, N, 1000, RooFit::EvalBackend::Legacy()); + std::tie(nll, pdf, data, values) = generate_ND_gaussian_pdf_nll(w, N, 1000, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)); likelihood = RFTS::buildLikelihood(pdf, data.get()); // dummy offsets (normally they are shared with other objects): SharedOffset offset; @@ -409,7 +409,7 @@ TEST_F(LikelihoodSerialSimBinnedConstrainedTest, ConstrainedAndOffset) // RooAbsTestStatistic. nll = std::unique_ptr{pdf->createNLL(*data, Constrain(*w.var("alpha_bkg_A")), GlobalObservables(*w.var("alpha_bkg_obs_B")), Offset("initial"), - EvalBackend::Legacy())}; + EvalBackend(EvalBackend::Value::Legacy))}; // -------- diff --git a/roofit/roofitcore/test/TestStatistics/testRooRealL.cxx b/roofit/roofitcore/test/TestStatistics/testRooRealL.cxx index 116a163823592..2e6099aba550a 100644 --- a/roofit/roofitcore/test/TestStatistics/testRooRealL.cxx +++ b/roofit/roofitcore/test/TestStatistics/testRooRealL.cxx @@ -212,7 +212,7 @@ TEST_P(RooRealL, setVal) // the multiprocess test statistics classes were designed to give values // that are bit-by-bit identical with the old test statistics based on // RooAbsTestStatistic. - std::unique_ptr nll{pdf->createNLL(*data, RooFit::EvalBackend::Legacy())}; + std::unique_ptr nll{pdf->createNLL(*data, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))}; RooFit::TestStatistics::RooRealL nll_new("nll_new", "new style NLL", std::make_unique(pdf, data.get())); @@ -270,7 +270,7 @@ TEST_P(RealLVsMPFE, getVal) // the multiprocess test statistics classes were designed to give values // that are bit-by-bit identical with the old test statistics based on // RooAbsTestStatistic. - std::unique_ptr nll_mpfe{pdf->createNLL(*data, RooFit::EvalBackend::Legacy())}; + std::unique_ptr nll_mpfe{pdf->createNLL(*data, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))}; auto mpfe_result = nll_mpfe->getVal(); diff --git a/roofit/roofitcore/test/gtest_wrapper.h b/roofit/roofitcore/test/gtest_wrapper.h index f342c55575ab8..a700a2ae877e3 100644 --- a/roofit/roofitcore/test/gtest_wrapper.h +++ b/roofit/roofitcore/test/gtest_wrapper.h @@ -13,7 +13,7 @@ #endif #ifdef ROOFIT_LEGACY_EVAL_BACKEND -#define ROOFIT_EVAL_BACKEND_LEGACY RooFit::EvalBackend::Legacy(), +#define ROOFIT_EVAL_BACKEND_LEGACY RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy), #else #define ROOFIT_EVAL_BACKEND_LEGACY #endif diff --git a/roofit/roofitcore/test/stressRooFit.cxx b/roofit/roofitcore/test/stressRooFit.cxx index 17bcfb842b589..6ac4072e36ecb 100644 --- a/roofit/roofitcore/test/stressRooFit.cxx +++ b/roofit/roofitcore/test/stressRooFit.cxx @@ -241,7 +241,7 @@ int main(int argc, const char *argv[]) int dryRun = false; bool doDump = false; bool doTreeStore = false; - auto backend = RooFit::EvalBackend::Legacy(); + auto backend = RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy); // string refFileName = "http://root.cern.ch/files/stressRooFit_v534_ref.root" ; string refFileName = "stressRooFit_ref.root"; diff --git a/roofit/roofitcore/test/testGlobalObservables.cxx b/roofit/roofitcore/test/testGlobalObservables.cxx index c9af30685f034..4b8265fbc95dc 100644 --- a/roofit/roofitcore/test/testGlobalObservables.cxx +++ b/roofit/roofitcore/test/testGlobalObservables.cxx @@ -46,7 +46,7 @@ bool isNotIdentical(RooFitResult const &res1, RooFitResult const &res2) // we track the global observables separately. class GlobsTest : public testing::TestWithParam> { public: - GlobsTest() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + GlobsTest() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} void SetUp() override { diff --git a/roofit/roofitcore/test/testNaNPacker.cxx b/roofit/roofitcore/test/testNaNPacker.cxx index c1f307ab60277..26688fdca014c 100644 --- a/roofit/roofitcore/test/testNaNPacker.cxx +++ b/roofit/roofitcore/test/testNaNPacker.cxx @@ -171,7 +171,7 @@ TEST(RooNaNPacker, FitSimpleLinear) class TestForDifferentBackends : public testing::TestWithParam> { public: - TestForDifferentBackends() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + TestForDifferentBackends() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} private: void SetUp() override diff --git a/roofit/roofitcore/test/testRooAbsPdf.cxx b/roofit/roofitcore/test/testRooAbsPdf.cxx index f13d2afa9e316..c5359a75d6f04 100644 --- a/roofit/roofitcore/test/testRooAbsPdf.cxx +++ b/roofit/roofitcore/test/testRooAbsPdf.cxx @@ -31,7 +31,7 @@ class FitTest : public testing::TestWithParam> { public: - FitTest() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + FitTest() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} private: void SetUp() override @@ -347,7 +347,7 @@ TEST_P(FitTest, MultiRangeFit2D) // If the BatchMode is off, we are doing the same cross-check also with the // chi-square fit on the RooDataHist. - if (_evalBackend.name() == EvalBackend::Legacy().name()) { + if (_evalBackend.name() == EvalBackend(EvalBackend::Value::Legacy).name()) { // full range resetValues(); @@ -467,7 +467,7 @@ TEST_P(FitTest, OutOfRangeDataThrows) // dataset's internal clone of the observable still remembers [0, 5]. x.setMax(2.5); - const bool isLegacy = _evalBackend == EvalBackend::Legacy(); + const bool isLegacy = _evalBackend == EvalBackend(EvalBackend::Value::Legacy); { // Normalizing over [0, 2.5] while still evaluating the entries at 3 would diff --git a/roofit/roofitcore/test/testRooBinSamplingPdf.cxx b/roofit/roofitcore/test/testRooBinSamplingPdf.cxx index 9a297225972ac..f4f2ff2c40a28 100644 --- a/roofit/roofitcore/test/testRooBinSamplingPdf.cxx +++ b/roofit/roofitcore/test/testRooBinSamplingPdf.cxx @@ -152,7 +152,7 @@ TEST(RooBinSamplingPdf, AnalyticalMatchesNumeric) std::unique_ptr dataH(gaus.generateBinned(x, 20000)); RooDataSet data("data", "data", x, RooFit::Import(*dataH)); - for (auto backend : {EvalBackend::Legacy(), EvalBackend::Cpu()}) { + for (auto backend : {EvalBackend(EvalBackend::Value::Legacy), EvalBackend::Cpu()}) { mean.setVal(0.7); sigma.setVal(1.3); std::unique_ptr nllAna{gaus.createNLL(data, IntegrateBins(1.E-3), backend)}; diff --git a/roofit/roofitcore/test/testRooMinimizer.cxx b/roofit/roofitcore/test/testRooMinimizer.cxx index 7494742178a7a..f565816adb474 100644 --- a/roofit/roofitcore/test/testRooMinimizer.cxx +++ b/roofit/roofitcore/test/testRooMinimizer.cxx @@ -14,7 +14,7 @@ class EvalBackendParametrizedTest : public testing::TestWithParam> { public: - EvalBackendParametrizedTest() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + EvalBackendParametrizedTest() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} private: void SetUp() override diff --git a/roofit/roofitcore/test/testRooProdPdf.cxx b/roofit/roofitcore/test/testRooProdPdf.cxx index d12c7d6c08438..88a33ba9da456 100644 --- a/roofit/roofitcore/test/testRooProdPdf.cxx +++ b/roofit/roofitcore/test/testRooProdPdf.cxx @@ -110,9 +110,9 @@ TEST(RooProdPdf, TestDepsAreCond) #ifdef ROOFIT_LEGACY_EVAL_BACKEND resetParameters(); - ResultPtr result1{pdf1.fitTo(*data, Save(), EvalBackend::Legacy(), PrintLevel(-1))}; + ResultPtr result1{pdf1.fitTo(*data, Save(), EvalBackend(EvalBackend::Value::Legacy), PrintLevel(-1))}; resetParameters(); - ResultPtr result3{pdf2.fitTo(*data, Save(), EvalBackend::Legacy(), PrintLevel(-1))}; + ResultPtr result3{pdf2.fitTo(*data, Save(), EvalBackend(EvalBackend::Value::Legacy), PrintLevel(-1))}; EXPECT_TRUE(result2->isIdentical(*result1)) << "legacy fit is inconsistent!"; EXPECT_TRUE(result4->isIdentical(*result1)) << "alternative model legacy fit is inconsistent!"; diff --git a/roofit/roofitcore/test/testRooSimultaneous.cxx b/roofit/roofitcore/test/testRooSimultaneous.cxx index 6792bd866fdf2..ca6a338d81b26 100644 --- a/roofit/roofitcore/test/testRooSimultaneous.cxx +++ b/roofit/roofitcore/test/testRooSimultaneous.cxx @@ -96,7 +96,7 @@ TEST(RooSimultaneous, CategoriesWithNoPdf) // We don't care about the fit result, just that it doesn't crash. using namespace RooFit; #ifdef ROOFIT_LEGACY_EVAL_BACKEND - sim.fitTo(*ds, EvalBackend::Legacy(), PrintLevel(-1)); + sim.fitTo(*ds, EvalBackend(EvalBackend::Value::Legacy), PrintLevel(-1)); m0.setVal(0.5); m0.setError(0.0); m1.setVal(0.5); @@ -148,7 +148,7 @@ TEST(RooSimultaneous, MultiRangeFitWithSplitRange) const char *cutRange2 = "SideBandLo_cat2,SideBandHi_cat2"; using RealPtr = std::unique_ptr; #ifdef ROOFIT_LEGACY_EVAL_BACKEND - RealPtr nllSim{simPdf.createNLL(combData, Range("SideBandLo,SideBandHi"), SplitRange(), EvalBackend::Legacy())}; + RealPtr nllSim{simPdf.createNLL(combData, Range("SideBandLo,SideBandHi"), SplitRange(), EvalBackend(EvalBackend::Value::Legacy))}; #endif RealPtr nllSimBatch{simPdf.createNLL(combData, Range("SideBandLo,SideBandHi"), SplitRange(), EvalBackend::Cpu())}; @@ -175,7 +175,7 @@ TEST(RooSimultaneous, MultiRangeFitWithSplitRange) class TestStatisticTest : public testing::TestWithParam> { public: - TestStatisticTest() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + TestStatisticTest() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} private: void SetUp() override @@ -868,7 +868,7 @@ TEST(RooSimultaneous, ParameterIndexSwitchMode) std::vector backends; #ifdef ROOFIT_LEGACY_EVAL_BACKEND - backends.push_back(RooFit::EvalBackend::Legacy()); + backends.push_back(RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)); #endif backends.push_back(RooFit::EvalBackend::Cpu()); backends.push_back(RooFit::EvalBackend::CodegenNoGrad()); @@ -946,7 +946,7 @@ TEST(RooSimultaneous, ParameterIndexTopLevelNLL) std::vector backends; #ifdef ROOFIT_LEGACY_EVAL_BACKEND - backends.push_back(RooFit::EvalBackend::Legacy()); + backends.push_back(RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)); #endif backends.push_back(RooFit::EvalBackend::Cpu()); backends.push_back(RooFit::EvalBackend::CodegenNoGrad()); diff --git a/roofit/roofitcore/test/testTestStatistics.cxx b/roofit/roofitcore/test/testTestStatistics.cxx index b9fbddcb864f3..3400e881acbab 100644 --- a/roofit/roofitcore/test/testTestStatistics.cxx +++ b/roofit/roofitcore/test/testTestStatistics.cxx @@ -66,7 +66,7 @@ std::unique_ptr generateBinnedAsimov(RooAbsPdf const &pdf, RooRealV class TestStatisticTest : public testing::TestWithParam> { public: - TestStatisticTest() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + TestStatisticTest() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} private: void SetUp() override @@ -408,13 +408,13 @@ TEST(RooChi2Var, ErrorTypesCrossCheck) // Chi2 value at a fixed parameter point should match to full precision. resetPars(); std::unique_ptr chi2New{gauss.createChi2(*hist, DataError(etype), backend)}; - std::unique_ptr chi2Legacy{gauss.createChi2(*hist, DataError(etype), EvalBackend::Legacy())}; + std::unique_ptr chi2Legacy{gauss.createChi2(*hist, DataError(etype), EvalBackend(EvalBackend::Value::Legacy))}; EXPECT_FLOAT_EQ(chi2New->getVal(), chi2Legacy->getVal()); // Minimisation should converge to the same minimum and parameter values. resetPars(); std::unique_ptr fitLegacy{ - gauss.chi2FitTo(*hist, DataError(etype), EvalBackend::Legacy(), Save(), PrintLevel(-1))}; + gauss.chi2FitTo(*hist, DataError(etype), EvalBackend(EvalBackend::Value::Legacy), Save(), PrintLevel(-1))}; resetPars(); std::unique_ptr fitNew{ gauss.chi2FitTo(*hist, DataError(etype), backend, Save(), PrintLevel(-1))}; @@ -447,7 +447,7 @@ TEST(RooChi2Var, ErrorTypesCrossCheck) RooRealVar nbkg("nbkg_func", "", 200., 0., 10000.); RooFormulaVar flat("flat", "flat", "nbkg_func + 0*x", {nbkg, x}); std::unique_ptr chi2Legacy{ - flat.createChi2(*hist, DataError(RooAbsData::Expected), EvalBackend::Legacy())}; + flat.createChi2(*hist, DataError(RooAbsData::Expected), EvalBackend(EvalBackend::Value::Legacy))}; for (auto const &backend : chi2CrossCheckBackends()) { SCOPED_TRACE(std::string("Function mode, backend = ") + backend.name()); std::unique_ptr chi2New{flat.createChi2(*hist, DataError(RooAbsData::Expected), backend)}; @@ -493,13 +493,13 @@ TEST(RooChi2Var, RangedCrossCheck) // Chi2 value at a fixed parameter point. resetPars(); std::unique_ptr chi2New{gauss.createChi2(*hist, Range(rangeName), backend)}; - std::unique_ptr chi2Legacy{gauss.createChi2(*hist, Range(rangeName), EvalBackend::Legacy())}; + std::unique_ptr chi2Legacy{gauss.createChi2(*hist, Range(rangeName), EvalBackend(EvalBackend::Value::Legacy))}; EXPECT_FLOAT_EQ(chi2New->getVal(), chi2Legacy->getVal()); // Fit comparison. resetPars(); std::unique_ptr fitLegacy{ - gauss.chi2FitTo(*hist, Range(rangeName), EvalBackend::Legacy(), Save(), PrintLevel(-1))}; + gauss.chi2FitTo(*hist, Range(rangeName), EvalBackend(EvalBackend::Value::Legacy), Save(), PrintLevel(-1))}; resetPars(); std::unique_ptr fitNew{ gauss.chi2FitTo(*hist, Range(rangeName), backend, Save(), PrintLevel(-1))}; @@ -562,7 +562,7 @@ TEST(RooChi2Var, SimultaneousCrossCheck) // Legacy baseline, computed once. resetPars(); - std::unique_ptr fitLegacy{simPdf.chi2FitTo(combHist, EvalBackend::Legacy(), Save(), PrintLevel(-1))}; + std::unique_ptr fitLegacy{simPdf.chi2FitTo(combHist, EvalBackend(EvalBackend::Value::Legacy), Save(), PrintLevel(-1))}; ASSERT_NE(fitLegacy, nullptr); for (auto const &backend : chi2CrossCheckBackends()) { @@ -571,7 +571,7 @@ TEST(RooChi2Var, SimultaneousCrossCheck) // Chi2 value at a fixed parameter point. resetPars(); std::unique_ptr chi2New{simPdf.createChi2(combHist, backend)}; - std::unique_ptr chi2Legacy{simPdf.createChi2(combHist, EvalBackend::Legacy())}; + std::unique_ptr chi2Legacy{simPdf.createChi2(combHist, EvalBackend(EvalBackend::Value::Legacy))}; EXPECT_FLOAT_EQ(chi2New->getVal(), chi2Legacy->getVal()); // Fit with the current backend, compare to the legacy baseline. @@ -610,8 +610,8 @@ TEST(RooNLLVar, CopyRangedNLL) // This bug is related to the implementation details of the old test // statistics, so the EvalBackend is forced to be Legacy using namespace RooFit; - std::unique_ptr nll{model.createNLL(*ds, EvalBackend::Legacy())}; - std::unique_ptr nllrange{model.createNLL(*ds, Range("fitrange"), EvalBackend::Legacy())}; + std::unique_ptr nll{model.createNLL(*ds, EvalBackend(EvalBackend::Value::Legacy))}; + std::unique_ptr nllrange{model.createNLL(*ds, Range("fitrange"), EvalBackend(EvalBackend::Value::Legacy))}; auto nllClone = std::make_unique(static_cast(*nll)); auto nllrangeClone = std::make_unique(static_cast(*nllrange)); @@ -624,7 +624,7 @@ TEST(RooNLLVar, CopyRangedNLL) class OffsetBinTest : public testing::TestWithParam> { public: - OffsetBinTest() : _evalBackend{RooFit::EvalBackend::Legacy()} {} + OffsetBinTest() : _evalBackend{RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy)} {} private: void SetUp() override @@ -982,6 +982,61 @@ TEST(NLL, SetData) // pdf. The RooFit logic to figure out constrained parameters should however // now be confused by this, and not strip away these parameters from the list // of constrained parameters. +#ifdef ROOFIT_LEGACY_EVAL_BACKEND +/// Check that selecting the deprecated legacy evaluation backend emits a +/// deprecation warning when the test statistic object is created. +TEST(CreateNLL, LegacyBackendDeprecationWarning) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::gauss(x[-10, 10], mean[0, -10, 10], sigma[2, 0.1, 10])"); + RooAbsPdf &gauss = *ws.pdf("gauss"); + std::unique_ptr data{gauss.generate(*ws.var("x"), 100)}; + std::unique_ptr hist{data->binnedClone()}; + + const std::string expectedSubstr = "deprecated and will be removed in ROOT 6.44"; + + { + RooHelpers::HijackMessageStream hijack(RooFit::WARNING, RooFit::InputArguments); + std::unique_ptr nll{ + gauss.createNLL(*data, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))}; + EXPECT_NE(hijack.str().find(expectedSubstr), std::string::npos) << hijack.str(); + } + + { + RooHelpers::HijackMessageStream hijack(RooFit::WARNING, RooFit::InputArguments); + std::unique_ptr chi2{ + gauss.createChi2(*hist, RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy))}; + EXPECT_NE(hijack.str().find(expectedSubstr), std::string::npos) << hijack.str(); + } + + // No warning must be emitted for the default backend. + { + RooHelpers::HijackMessageStream hijack(RooFit::WARNING, RooFit::InputArguments); + std::unique_ptr nll{gauss.createNLL(*data)}; + EXPECT_EQ(hijack.str().find(expectedSubstr), std::string::npos) << hijack.str(); + } +} +#endif // ROOFIT_LEGACY_EVAL_BACKEND + +/// The deprecated BatchMode() command argument must still map to the right +/// evaluation backends and emit a deprecation warning. This also guards +/// against the C++ declarations going missing again, like they accidentally +/// did between ROOT 6.30 and 6.40. +TEST(CreateNLL, BatchModeDeprecationWarning) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" + RooHelpers::HijackMessageStream hijack(RooFit::WARNING, RooFit::InputArguments); + EXPECT_EQ(RooFit::BatchMode("cpu").getInt(0), static_cast(RooFit::EvalBackend::Value::Cpu)); + EXPECT_EQ(RooFit::BatchMode("off").getInt(0), static_cast(RooFit::EvalBackend::Value::Legacy)); + EXPECT_EQ(RooFit::BatchMode(true).getInt(0), static_cast(RooFit::EvalBackend::Value::Cpu)); + EXPECT_NE(hijack.str().find("deprecated and will be removed in ROOT 6.44"), std::string::npos) << hijack.str(); +#pragma GCC diagnostic pop +} + TEST(CreateNLL, CombineStyleConstraints) { RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); diff --git a/roofit/roostats/test/stressRooStats.cxx b/roofit/roostats/test/stressRooStats.cxx index 0dbd92997536c..916fb9348eea7 100644 --- a/roofit/roostats/test/stressRooStats.cxx +++ b/roofit/roostats/test/stressRooStats.cxx @@ -301,7 +301,7 @@ int main(int argc, const char *argv[]) bool dryRun = false; bool doDump = false; bool doTreeStore = false; - auto backend = RooFit::EvalBackend::Legacy(); + auto backend = RooFit::EvalBackend(RooFit::EvalBackend::Value::Legacy); // string refFileName = "http://root.cern/files/stressRooStats_v534_ref.root" ; string refFileName = "stressRooStats_ref.root";