From 0a9f78d9f6b11b323c35a639fc63872d5f6e4c96 Mon Sep 17 00:00:00 2001 From: Jonas Rembser Date: Mon, 4 Dec 2023 11:19:54 +0100 Subject: [PATCH] Don't do public accessor workarounds if not needed Some public accessor hacks are now hidden behind preprocessor macros. This will serve as a reminder to remove these workarounds in the far future when combine doesn't support old ROOT versions anymore. --- interface/utils.h | 2 ++ src/CachingMultiPdf.cc | 4 ++++ src/VectorizedHistFactoryPdfs.cc | 16 ++++++++++++---- src/utils.cc | 19 ++++++++++++++++--- 4 files changed, 34 insertions(+), 7 deletions(-) diff --git a/interface/utils.h b/interface/utils.h index 04490a89967..0cf66b8682b 100644 --- a/interface/utils.h +++ b/interface/utils.h @@ -56,8 +56,10 @@ namespace utils { /// factorize a RooAbsReal void factorizeFunc(const RooArgSet &observables, RooAbsReal &pdf, RooArgList &obsTerms, RooArgList &otherTerms, bool keepDuplicates = true, bool debug=false); +#if ROOT_VERSION_CODE < ROOT_VERSION(6,28,0) /// workaround for RooProdPdf::components() RooArgList factors(const RooProduct &prod) ; +#endif /// Note: doesn't recompose Simultaneous pdfs properly, for that use factorizePdf method RooAbsPdf *makeObsOnlyPdf(RooStats::ModelConfig &model, const char *name="obsPdf") ; diff --git a/src/CachingMultiPdf.cc b/src/CachingMultiPdf.cc index bbc83a67843..eb7840d46e2 100644 --- a/src/CachingMultiPdf.cc +++ b/src/CachingMultiPdf.cc @@ -134,7 +134,11 @@ void cacheutils::CachingAddPdf::setIncludeZeroWeights(bool includeZeroWeights) cacheutils::CachingProduct::CachingProduct(const RooProduct &pdf, const RooArgSet &obs) : pdf_(&pdf) { +#if ROOT_VERSION_CODE < ROOT_VERSION(6,28,0) const RooArgList & pdfs = utils::factors(pdf); +#else + const RooArgList & pdfs = pdf.realComponents(); +#endif //std::cout << "Making a CachingProduct for " << pdf.GetName() << " with " << pdfs.getSize() << " pdfs, " << coeffs.getSize() << " coeffs." << std::endl; for (int i = 0, n = pdfs.getSize(); i < n; ++i) { RooAbsReal *pdfi = (RooAbsReal*) pdfs.at(i); diff --git a/src/VectorizedHistFactoryPdfs.cc b/src/VectorizedHistFactoryPdfs.cc index eae5d4ea59e..83874d5764b 100644 --- a/src/VectorizedHistFactoryPdfs.cc +++ b/src/VectorizedHistFactoryPdfs.cc @@ -81,24 +81,32 @@ cacheutils::VectorizedParamHistFunc::fill(std::vector &out) const } } +#if ROOT_VERSION_CODE < ROOT_VERSION(6,30,0) namespace { class PiecewiseInterpolationWithAccessor : public PiecewiseInterpolation { public: PiecewiseInterpolationWithAccessor(const PiecewiseInterpolation &p) : PiecewiseInterpolation(p) {} - const RooAbsReal & nominal() const { return _nominal.arg(); } - int getInterpCode(int i) const { return _interpCode[i]; } + const RooAbsReal* nominalHist() const { return &_nominal.arg(); } + const std::vector& interpolationCodes() const { return _interpCode; } bool positiveDefinite() const { return _positiveDefinite; } }; } +#endif cacheutils::CachingPiecewiseInterpolation::CachingPiecewiseInterpolation(const PiecewiseInterpolation &pdf, const RooArgSet &obs) : pdf_(&pdf) { +#if ROOT_VERSION_CODE < ROOT_VERSION(6,30,0) + // Since ROOT 6.30, the public accessors are in upstream RooFit and we + // don't need this hack. PiecewiseInterpolationWithAccessor fixme(pdf); +#else + auto& fixme = pdf; +#endif const RooArgList & highList = pdf.highList(); const RooArgList & lowList = pdf.lowList(); - const RooAbsReal & nominal = fixme.nominal(); + const RooAbsReal & nominal = *fixme.nominalHist(); const RooArgList & coeffs = pdf.paramList(); //std::cout << "Making a CachingPiecewiseInterpolation for " << pdf.GetName() << " with " << highList.getSize() << " pdfs, " << coeffs.getSize() << " coeffs." << std::endl; positiveDefinite_ = fixme.positiveDefinite(); @@ -109,7 +117,7 @@ cacheutils::CachingPiecewiseInterpolation::CachingPiecewiseInterpolation(const P RooAbsReal *pdfiLo = (RooAbsReal*) lowList.at(i); cachingPdfsLow_.push_back(makeCachingPdf(pdfiLo, &obs)); coeffs_.push_back((RooAbsReal*) coeffs.at(i)); - codes_.push_back(fixme.getInterpCode(i)); + codes_.push_back(fixme.interpolationCodes()[i]); //std::cout << " PiecewiseInterpolation Adding " << pdf.GetName() << "[" << i << "] Hi : " << pdfiHi->ClassName() << " " << pdfiHi->GetName() << " using " << typeid(cachingPdfsHi_.back()).name() << std::endl; //std::cout << " PiecewiseInterpolation Adding " << pdf.GetName() << "[" << i << "] Hi : " << pdfiHi->ClassName() << " " << pdfiHi->GetName() << " using " << typeid(cachingPdfsHi_.back()).name() << std::endl; //std::cout << " PiecewiseInterpolation Adding " << pdf.GetName() << "[" << i << "] Coeff : " << coeffs_.back()->ClassName() << " " << coeffs_.back()->GetName() << std::endl; diff --git a/src/utils.cc b/src/utils.cc index df4a53c2d04..f647f1b50b6 100644 --- a/src/utils.cc +++ b/src/utils.cc @@ -43,7 +43,13 @@ using namespace std; +#if ROOT_VERSION_CODE < ROOT_VERSION(6,28,0) + // This is needed to be able to factorize products in factorizeFunc, since RooProduct::components() strips duplicates +// +// Note: since ROOT 6.28.00, this workaround is not necessary more, because +// there is direct public access to the _compRSet via +// RooProduct::realComponents(). namespace { class RooProductWithAccessors : public RooProduct { public: @@ -61,6 +67,12 @@ namespace { }; } +RooArgList utils::factors(const RooProduct &prod) { + return ::RooProductWithAccessors(prod).realTerms(); +} + +#endif + void utils::printRDH(RooAbsData *data) { std::vector varnames, catnames; const RooArgSet *b0 = data->get(); @@ -224,9 +236,6 @@ void utils::factorizePdf(const RooArgSet &observables, RooAbsPdf &pdf, RooArgLis } -RooArgList utils::factors(const RooProduct &prod) { - return ::RooProductWithAccessors(prod).realTerms(); -} void utils::factorizeFunc(const RooArgSet &observables, RooAbsReal &func, RooArgList &obsTerms, RooArgList &constraints, bool keepDuplicate, bool debug) { RooAbsPdf *pdf = dynamic_cast(&func); if (pdf != 0) { @@ -236,7 +245,11 @@ void utils::factorizeFunc(const RooArgSet &observables, RooAbsReal &func, RooArg const std::type_info & id = typeid(func); if (id == typeid(RooProduct)) { RooProduct *prod = dynamic_cast(&func); +#if ROOT_VERSION_CODE < ROOT_VERSION(6,28,0) RooArgList components(utils::factors(*prod)); +#else + RooArgList components(prod->realComponents()); +#endif //std::cout << "Function " << func.GetName() << " is a RooProduct with " << components.getSize() << " components." << std::endl; std::unique_ptr iter(components.createIterator()); for (RooAbsReal *funci = (RooAbsReal *) iter->Next(); funci != 0; funci = (RooAbsReal *) iter->Next()) {