Skip to content

Commit

Permalink
Don't do public accessor workarounds if not needed (#875)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
guitargeek authored Dec 20, 2023
1 parent 2763577 commit 7a408dd
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 7 deletions.
2 changes: 2 additions & 0 deletions interface/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -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") ;
Expand Down
4 changes: 4 additions & 0 deletions src/CachingMultiPdf.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
16 changes: 12 additions & 4 deletions src/VectorizedHistFactoryPdfs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,24 +81,32 @@ cacheutils::VectorizedParamHistFunc::fill(std::vector<Double_t> &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<int>& 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();
Expand All @@ -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;
Expand Down
19 changes: 16 additions & 3 deletions src/utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -61,6 +67,12 @@ namespace {
};
}

RooArgList utils::factors(const RooProduct &prod) {
return ::RooProductWithAccessors(prod).realTerms();
}

#endif

void utils::printRDH(RooAbsData *data) {
std::vector<std::string> varnames, catnames;
const RooArgSet *b0 = data->get();
Expand Down Expand Up @@ -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<RooAbsPdf *>(&func);
if (pdf != 0) {
Expand All @@ -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<RooProduct *>(&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<TIterator> iter(components.createIterator());
for (RooAbsReal *funci = (RooAbsReal *) iter->Next(); funci != 0; funci = (RooAbsReal *) iter->Next()) {
Expand Down

0 comments on commit 7a408dd

Please sign in to comment.