diff --git a/roofit/roofitcore/inc/RooAddModel.h b/roofit/roofitcore/inc/RooAddModel.h index d279aa3a89db8..216b168c874c9 100644 --- a/roofit/roofitcore/inc/RooAddModel.h +++ b/roofit/roofitcore/inc/RooAddModel.h @@ -99,7 +99,7 @@ class RooAddModel : public RooResolutionModel { mutable RooObjCacheManager _projCacheMgr ; ///selfNormalized(); } + bool selfNormalized() const override { return true; } /// Forwards to the PDF's implementation. RooAbsReal* createIntegral(const RooArgSet& iset, diff --git a/roofit/roofitcore/inc/RooHelpers.h b/roofit/roofitcore/inc/RooHelpers.h index 30d0430e22ef4..12b0908b0545c 100644 --- a/roofit/roofitcore/inc/RooHelpers.h +++ b/roofit/roofitcore/inc/RooHelpers.h @@ -132,10 +132,9 @@ class ChangeOperModeRAII { std::pair getRangeOrBinningInterval(RooAbsArg const* arg, const char* rangeName); -bool checkIfRangesOverlap(RooAbsPdf const& pdf, +bool checkIfRangesOverlap(RooArgSet const& observables, RooAbsData const& data, - std::vector const& rangeNames, - bool splitRange); + std::vector const& rangeNames); std::string getColonSeparatedNameString(RooArgSet const& argSet); RooArgSet selectFromArgSet(RooArgSet const&, std::string const& names); diff --git a/roofit/roofitcore/res/RooNLLVarNew.h b/roofit/roofitcore/res/RooNLLVarNew.h index 9b815fa2b0db4..fa588b3958581 100644 --- a/roofit/roofitcore/res/RooNLLVarNew.h +++ b/roofit/roofitcore/res/RooNLLVarNew.h @@ -34,7 +34,7 @@ class RooNLLVarNew : public RooAbsReal { RooNLLVarNew(){}; RooNLLVarNew(const char *name, const char *title, RooAbsPdf &pdf, RooArgSet const &observables, bool isExtended, - std::string const &rangeName, bool doOffset); + bool doOffset); RooNLLVarNew(const RooNLLVarNew &other, const char *name = nullptr); TObject *clone(const char *newname) const override { return new RooNLLVarNew(*this, newname); } @@ -68,7 +68,6 @@ class RooNLLVarNew : public RooAbsReal { std::string _prefix; RooTemplateProxy _weightVar; RooTemplateProxy _weightSquaredVar; - std::unique_ptr> _fractionInRange; mutable std::vector _binw; /// _logProbasBuffer; /// _offset = 0.0; /// createSimultaneousNLL(RooSimultaneous const &simPdf, auto const &catName = catItem.first; if (RooAbsPdf *pdf = simPdf.getPdf(catName.c_str())) { auto name = std::string("nll_") + pdf->GetName(); - auto nll = std::make_unique(name.c_str(), name.c_str(), *pdf, observables, isExtended, rangeName, - doOffset); + if (!rangeName.empty()) { + pdf->setNormRange(rangeName.c_str()); + } + auto nll = std::make_unique(name.c_str(), name.c_str(), *pdf, observables, isExtended, doOffset); // Rename the observables and weights newObservables.add(nll->prefixObservableAndWeightNames(std::string("_") + catName + "_")); nllTerms.addOwned(std::move(nll)); @@ -152,6 +154,11 @@ RooFit::BatchModeHelpers::createNLL(RooAbsPdf &pdf, RooAbsData &data, std::uniqu observables.remove(projDeps, true, true); obsClones.remove(projDeps, true, true); + // Set the normalization range + if (!rangeName.empty()) { + pdfClone->setNormRange(rangeName.c_str()); + } + oocxcoutI(&pdf, Fitting) << "RooAbsPdf::fitTo(" << pdf.GetName() << ") fixing normalization set for coefficient determination to observables in data" << "\n"; @@ -183,8 +190,8 @@ RooFit::BatchModeHelpers::createNLL(RooAbsPdf &pdf, RooAbsData &data, std::uniqu // Warning! This mutates "obsClones" nllTerms.addOwned(createSimultaneousNLL(*simPdf, obsClones, isExtended, rangeName, doOffset)); } else { - nllTerms.addOwned(std::make_unique("RooNLLVarNew", "RooNLLVarNew", finalPdf, obsClones, isExtended, - rangeName, doOffset)); + nllTerms.addOwned( + std::make_unique("RooNLLVarNew", "RooNLLVarNew", finalPdf, obsClones, isExtended, doOffset)); } if (constraints) { nllTerms.addOwned(std::move(constraints)); diff --git a/roofit/roofitcore/src/RooAbsData.cxx b/roofit/roofitcore/src/RooAbsData.cxx index c8d9e9d08a1d7..dfef90dbbbc79 100644 --- a/roofit/roofitcore/src/RooAbsData.cxx +++ b/roofit/roofitcore/src/RooAbsData.cxx @@ -498,13 +498,9 @@ RooAbsData* RooAbsData::reduce(const RooCmdArg& arg1,const RooCmdArg& arg2,const RooFormulaVar cutVarTmp(cutSpec,cutSpec,*get()) ; ret = reduceEng(varSubset,&cutVarTmp,cutRange,nStart,nStop) ; - } else if (cutVar) { - - ret = reduceEng(varSubset,cutVar,cutRange,nStart,nStop) ; - } else { - ret = reduceEng(varSubset,0,cutRange,nStart,nStop) ; + ret = reduceEng(varSubset,cutVar,cutRange,nStart,nStop) ; } diff --git a/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx b/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx index 0246c3e3f086c..f2d63cb7e4205 100644 --- a/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx +++ b/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx @@ -61,6 +61,8 @@ parallelized calculation of test statistics. #include "RooVectorDataStore.h" #include "RooBinSamplingPdf.h" +#include "ROOT/StringUtils.hxx" + using namespace std; ClassImp(RooAbsOptTestStatistic); @@ -290,16 +292,27 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con // ****************************************************************** std::unique_ptr origObsSet( real.getObservables(indata) ); - RooArgSet* dataObsSet = (RooArgSet*) _dataClone->get() ; if (rangeName && strlen(rangeName)) { cxcoutI(Fitting) << "RooAbsOptTestStatistic::ctor(" << GetName() << ") constructing test statistic for sub-range named " << rangeName << endl ; - bool observablesKnowRange = false; + if(auto pdfClone = dynamic_cast(_funcClone)) { + pdfClone->setNormRange(rangeName); + } + // Adjust FUNC normalization ranges to requested fitRange, store original ranges for RooAddPdf coefficient interpretation for (const auto arg : *_funcObsSet) { - RooRealVar* realObs = dynamic_cast(arg) ; - if (realObs) { + if (auto realObs = dynamic_cast(arg)) { + + auto tokens = ROOT::Split(rangeName, ","); + for(std::string const& token : tokens) { + if(!realObs->hasRange(token.c_str())) { + std::stringstream errMsg; + errMsg << "The observable \"" << realObs->GetName() << "\" doesn't define the requested range \"" + << token << "\". Replacing it with the default range." << std::endl; + coutW(Fitting) << errMsg.str() << std::endl; + } + } auto transferRangeAndBinning = [&](RooRealVar & toVar, const char* toName, const char* fromName) { toVar.setRange(toName, realObs->getMin(fromName),realObs->getMax(fromName)); @@ -312,22 +325,11 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con } }; - observablesKnowRange |= realObs->hasRange(rangeName); - - // If no explicit range is given for RooAddPdf coefficients, create explicit named range equivalent to original observables range - if (!(addCoefRangeName && strlen(addCoefRangeName))) { - transferRangeAndBinning(*realObs, Form("NormalizationRangeFor%s",rangeName), nullptr); - } - - // Adjust range of function observable to those of given named range - transferRangeAndBinning(*realObs, nullptr, rangeName); - - // Adjust range of data observable to those of given named range - RooRealVar* dataObs = (RooRealVar*) dataObsSet->find(realObs->GetName()) ; - transferRangeAndBinning(*dataObs, nullptr, rangeName); - - // Keep track of list of fit ranges in string attribute fit range of original p.d.f. - if (!_splitRange) { + // Keep track of list of fit ranges in string attribute fit range of + // original PDF, but only do so if we don't do multi-range + // normalization. In this case, we could not define an equivalend + // single fit range. + if (!_splitRange && strchr(rangeName,',') == 0) { const std::string fitRangeName = std::string("fit_") + GetName(); const char* origAttrib = real.getStringAttribute("fitrange") ; std::string newAttr = origAttrib ? origAttrib : ""; @@ -343,9 +345,6 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con } } } - - if (!observablesKnowRange) - coutW(Fitting) << "None of the fit observables seem to know the range '" << rangeName << "'. This means that the full range will be used." << std::endl; } @@ -366,10 +365,6 @@ void RooAbsOptTestStatistic::initSlave(RooAbsReal& real, RooAbsData& indata, con cxcoutI(Fitting) << "RooAbsOptTestStatistic::ctor(" << GetName() << ") fixing interpretation of coefficients of any RooAddPdf component to range " << addCoefRangeName << endl ; _funcClone->fixAddCoefRange(addCoefRangeName,false) ; - } else { - cxcoutI(Fitting) << "RooAbsOptTestStatistic::ctor(" << GetName() - << ") fixing interpretation of coefficients of any RooAddPdf to full domain of observables " << endl ; - _funcClone->fixAddCoefRange(Form("NormalizationRangeFor%s",rangeName),false) ; } } @@ -859,3 +854,4 @@ void RooAbsOptTestStatistic::setUpBinSampling() { const char* RooAbsOptTestStatistic::cacheUniqueSuffix() const { return Form("_%lx", _dataClone->uniqueId().value()) ; } + diff --git a/roofit/roofitcore/src/RooAbsPdf.cxx b/roofit/roofitcore/src/RooAbsPdf.cxx index 855ac6f1fdbb1..2a68a7d3b11e5 100644 --- a/roofit/roofitcore/src/RooAbsPdf.cxx +++ b/roofit/roofitcore/src/RooAbsPdf.cxx @@ -938,51 +938,6 @@ RooAbsReal* RooAbsPdf::createNLL(RooAbsData& data, const RooCmdArg& arg1, const return createNLL(data,l) ; } -namespace { - -std::unique_ptr createMultiRangeNLLCorrectionTerm( - RooAbsPdf const &pdf, RooAbsData const &data, std::string const &baseName, std::string const &rangeNames) -{ - double sumEntriesTotal = 0.0; - - RooArgList termList; - RooArgList integralList; - - for (const auto ¤tRangeName : ROOT::Split(rangeNames, ",")) { - const std::string currentName = baseName + "_" + currentRangeName; - - auto sumEntriesCurrent = data.sumEntries("1", currentRangeName.c_str()); - sumEntriesTotal += sumEntriesCurrent; - - RooArgSet depList; - pdf.getObservables(data.get(), depList); - auto pdfIntegralCurrent = pdf.createIntegral(depList, &depList, nullptr, currentRangeName.c_str()); - - auto term = new RooFormulaVar((currentName + "_correctionTerm").c_str(), - (std::string("-(") + std::to_string(sumEntriesCurrent) + " * log(x[0]))").c_str(), - RooArgList(*pdfIntegralCurrent)); - - termList.add(*term); - integralList.add(*pdfIntegralCurrent); - } - - auto integralFull = new RooAddition((baseName + "_correctionFullIntegralTerm").c_str(), - "integral", - integralList, - true); - - auto fullRangeTerm = new RooFormulaVar((baseName + "_foobar").c_str(), - (std::string("(") + std::to_string(sumEntriesTotal) + " * log(x[0]))").c_str(), - RooArgList(*integralFull)); - - termList.add(*fullRangeTerm); - return std::unique_ptr{ - new RooAddition((baseName + "_correction").c_str(), "correction", termList, true)}; -} - - -} // namespace - //////////////////////////////////////////////////////////////////////////////// /// Construct representation of -log(L) of PDFwith given dataset. If dataset is unbinned, an unbinned likelihood is constructed. If the dataset @@ -1129,43 +1084,9 @@ RooAbsReal* RooAbsPdf::createNLL(RooAbsData& data, const RooLinkedList& cmdList) cfg.integrateOverBinsPrecision = pc.getDouble("IntegrateBins"); cfg.binnedL = false; cfg.takeGlobalObservablesFromData = takeGlobalObservablesFromData; - if (!rangeName || strchr(rangeName,',')==0) { - // Simple case: default range, or single restricted range - //cout<<"FK: Data test 1: "<(baseName.c_str(),"-log(likelihood)",*this,data,projDeps, ext, cfg); - static_cast(*nll).batchMode(batchMode == RooFit::BatchModeOption::Old); - } else { - // Composite case: multiple ranges - RooArgList nllList ; - auto tokens = ROOT::Split(rangeName, ","); - if (RooHelpers::checkIfRangesOverlap(*this, data, tokens, cfg.splitCutRange)) { - throw std::runtime_error( - std::string("Error in RooAbsPdf::createNLL! The ranges ") + rangeName + " are overlapping!"); - } - for (const auto& token : tokens) { - cfg.rangeName = token; - auto nllComp = std::make_unique((baseName + "_" + token).c_str(),"-log(likelihood)", - *this,data,projDeps,ext,cfg); - nllComp->batchMode(pc.getInt("BatchMode")); - nllList.addOwned(std::move(nllComp)) ; - } - - if (!ext) { - // Each RooNLLVar was created with the normalization set corresponding to - // the subrange, not the union range like it should be. We have to add an - // extra term to cancel this normalization problem. However, this is - // only necessarry for the non-extended case, because adding an extension - // term to the individual NLLs as done here is mathematicall equivalent - // to adding the normalization correction terms plus a global extension - // term. - nllList.addOwned(createMultiRangeNLLCorrectionTerm(*this, data, baseName, rangeName)); - } - - nll = std::make_unique(baseName.c_str(),"-log(likelihood)",nllList) ; - nll->addOwnedComponents(std::move(nllList)); - } + cfg.rangeName = rangeName ? rangeName : ""; + nll = std::make_unique(baseName.c_str(),"-log(likelihood)",*this,data,projDeps, ext, cfg); + static_cast(*nll).batchMode(batchMode == RooFit::BatchModeOption::Old); RooAbsReal::setEvalErrorLoggingMode(RooAbsReal::PrintErrors) ; // Include constraints, if any, in likelihood diff --git a/roofit/roofitcore/src/RooAbsTestStatistic.cxx b/roofit/roofitcore/src/RooAbsTestStatistic.cxx index 145016755a7a5..7f3522058e3a3 100644 --- a/roofit/roofitcore/src/RooAbsTestStatistic.cxx +++ b/roofit/roofitcore/src/RooAbsTestStatistic.cxx @@ -49,6 +49,8 @@ combined in the main thread. #include "RooRealSumPdf.h" #include "RooAbsCategoryLValue.h" +#include "ROOT/StringUtils.hxx" + #include "TTimeStamp.h" #include "TClass.h" #include @@ -107,16 +109,6 @@ RooAbsTestStatistic::RooAbsTestStatistic(const char *name, const char *title, Ro { // Register all parameters as servers _paramSet.add(*std::unique_ptr{real.getParameters(&data)}); - - if (cfg.rangeName.find(',') != std::string::npos) { - auto errorMsg = std::string("Ranges ") + cfg.rangeName - + " were passed to the RooAbsTestStatistic with name \"" + name + "\", " - + "but it doesn't support multiple comma-separated fit ranges!\n" + - + "Instead, one should combine multiple RooAbsTestStatistic objects " - + "(see RooAbsPdf::createNLL for an example with RooNLLVar)."; - coutE(InputArguments) << errorMsg << std::endl; - throw std::invalid_argument(errorMsg); - } } @@ -535,7 +527,11 @@ void RooAbsTestStatistic::initSimMode(RooSimultaneous* simpdf, RooAbsData* data, cfg.integrateOverBinsPrecision = thisAsRooAbsOptTestStatistic->_integrateBinsPrecision; } if (_splitRange && !rangeName.empty()) { - cfg.rangeName = rangeName + "_" + catName; + auto tokens = ROOT::Split(rangeName, ","); + for(std::string const& token : tokens) { + cfg.rangeName += token + "_" + catName + ","; + } + cfg.rangeName.pop_back(); // to remove the last comma cfg.nCPU = _nCPU*(_mpinterl?-1:1); } else { cfg.rangeName = rangeName; diff --git a/roofit/roofitcore/src/RooAddGenContext.cxx b/roofit/roofitcore/src/RooAddGenContext.cxx index 732efc952e5cc..29105065123a6 100644 --- a/roofit/roofitcore/src/RooAddGenContext.cxx +++ b/roofit/roofitcore/src/RooAddGenContext.cxx @@ -150,7 +150,7 @@ void RooAddGenContext::initGenerator(const RooArgSet &theEvent) _pcache = amod->getProjCache(_vars.get()) ; } else { RooAddPdf* apdf = (RooAddPdf*) _pdf ; - _pcache = apdf->getProjCache(_vars.get(),nullptr,"FULL_RANGE_ADDGENCONTEXT") ; + _pcache = apdf->getProjCache(_vars.get(),nullptr) ; } // Forward initGenerator call to all components diff --git a/roofit/roofitcore/src/RooAddHelpers.cxx b/roofit/roofitcore/src/RooAddHelpers.cxx index f59668d09670c..590eea730b667 100644 --- a/roofit/roofitcore/src/RooAddHelpers.cxx +++ b/roofit/roofitcore/src/RooAddHelpers.cxx @@ -16,15 +16,20 @@ #include #include #include +#include //////////////////////////////////////////////////////////////////////////////// /// Create a RooAddPdf cache element for a given normalization set and /// projection configuration. AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, RooArgList const &coefList, - const RooArgSet *nset, const RooArgSet *iset, const char *rangeName, bool projectCoefs, - RooArgSet const &refCoefNorm, TNamed const *refCoefRangeName, int verboseEval) + const RooArgSet *nset, const RooArgSet *iset, bool projectCoefs, + RooArgSet const &refCoefNormSet, std::string const &refCoefNormRange, int verboseEval) { + // We put the normRange into a std::string to not have to deal with + // nullptr vs. "" ambiguities + const std::string normRange = addPdf.normRange() ? addPdf.normRange() : ""; + _suppNormList.reserve(pdfList.size()); // *** PART 1 : Create supplemental normalization list *** @@ -36,11 +41,15 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R fullDepList.remove(*iset, true, true); } + bool hasPdfWithCustomRange = false; + // Fill with dummy unit RRVs for now for (std::size_t i = 0; i < pdfList.size(); ++i) { auto pdf = static_cast(pdfList.at(i)); auto coef = static_cast(coefList.at(i)); + hasPdfWithCustomRange |= pdf->normRange() != nullptr; + // Start with full list of dependents RooArgSet supNSet(fullDepList); @@ -63,6 +72,17 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R << " making supplemental normalization set " << supNSet << " for pdf component " << pdf->GetName() << std::endl; } + + if (!normRange.empty()) { + auto snormTerm = std::unique_ptr(pdf->createIntegral(*nset, *nset, normRange.c_str())); + if (snorm) { + auto oldSnorm = std::move(snorm); + snorm = std::make_unique("snorm", "snorm", *oldSnorm.get(), *snormTerm.get()); + snorm->addOwnedComponents(std::move(snormTerm), std::move(oldSnorm)); + } else { + snorm = std::move(snormTerm); + } + } _suppNormList.emplace_back(std::move(snorm)); } @@ -74,12 +94,8 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R // *** PART 2 : Create projection coefficients *** - // cout << " this = " << this << " (" << GetName() << ")" << std::endl ; - // cout << "projectCoefs = " << (_projectCoefs?"T":"F") << std::endl ; - // cout << "_normRange.Length() = " << _normRange.Length() << std::endl ; - // If no projections required stop here - if (!projectCoefs && !rangeName) { + if (!projectCoefs && !hasPdfWithCustomRange) { // cout << " no projection required" << std::endl ; return; } @@ -94,36 +110,26 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R << ")::getPC nset = " << (nset ? *nset : RooArgSet()) << " nset2 = " << nset2 << std::endl; - if (nset2.empty() && !refCoefNorm.empty()) { + if (nset2.empty() && !refCoefNormSet.empty()) { // cout << "WVE: evaluating RooAddPdf without normalization, but have reference normalization for coefficient // definition" << std::endl ; - nset2.add(refCoefNorm); - if (refCoefRangeName) { - rangeName = RooNameReg::str(refCoefRangeName); - } + nset2.add(refCoefNormSet); } - // Check if requested transformation is not identity - if (!nset2.equals(refCoefNorm) || refCoefRangeName != 0 || rangeName != 0 || addPdf.normRange()) { - - oocxcoutD(&addPdf, Caching) << "ALEX: " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName() - << ") projecting coefficients from " << nset2 << (rangeName ? ":" : "") - << (rangeName ? rangeName : "") << " to " - << ((!refCoefNorm.empty()) ? refCoefNorm : nset2) << (refCoefRangeName ? ":" : "") - << (refCoefRangeName ? RooNameReg::str(refCoefRangeName) : "") << std::endl; + if (!nset2.equals(refCoefNormSet) || !refCoefNormRange.empty() || !normRange.empty() || hasPdfWithCustomRange) { // Recalculate projection integrals of PDFs for (auto *pdf : static_range_cast(pdfList)) { // Calculate projection integral std::unique_ptr pdfProj; - if (!nset2.equals(refCoefNorm)) { - pdfProj = std::unique_ptr{pdf->createIntegral(nset2, refCoefNorm, addPdf.normRange())}; + if (!refCoefNormSet.empty() && !nset2.equals(refCoefNormSet)) { + pdfProj = std::unique_ptr{pdf->createIntegral(nset2, refCoefNormSet, normRange.c_str())}; pdfProj->setOperMode(addPdf.operMode()); oocxcoutD(&addPdf, Caching) << addPdf.ClassName() << "(" << addPdf.GetName() << ")::getPC nset2(" << nset2 - << ")!=_refCoefNorm(" << refCoefNorm << ") --> pdfProj = " << pdfProj->GetName() - << std::endl; + << ")!=_refCoefNormSet(" << refCoefNormSet + << ") --> pdfProj = " << pdfProj->GetName() << std::endl; oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName() << ") PP = " << pdfProj->GetName() << std::endl; } @@ -131,13 +137,13 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R _projList.emplace_back(std::move(pdfProj)); // Calculation optional supplemental normalization term - RooArgSet supNormSet(refCoefNorm); + RooArgSet supNormSet(refCoefNormSet); auto deps = std::unique_ptr{pdf->getParameters(RooArgSet())}; supNormSet.remove(*deps, true, true); std::unique_ptr snorm; auto name = std::string(addPdf.GetName()) + "_" + pdf->GetName() + "_ProjSupNorm"; - if (!supNormSet.empty() && !nset2.equals(refCoefNorm)) { + if (!supNormSet.empty() && !nset2.equals(refCoefNormSet)) { snorm = std::make_unique(name.c_str(), "Projection Supplemental normalization integral", RooRealConstant::value(1.0), supNormSet); oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName() @@ -145,67 +151,41 @@ AddCacheElem::AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, R } _suppProjList.emplace_back(std::move(snorm)); - // Calculate reference range adjusted projection integral - std::unique_ptr rangeProj1; - - // cout << "ALEX >>>> RooAddPdf(" << GetName() << ")::getPC refCoefRangeName WVE = " - // <<(refCoefRangeName?":":"") << (refCoefRangeName?RooNameReg::str(refCoefRangeName):"") - // <<" refCoefRangeName AK = " << (refCoefRangeName?refCoefRangeName->GetName():"") - // << " && _refCoefNorm" << _refCoefNorm << " with size = _refCoefNorm.size() " << _refCoefNorm.size() << - // std::endl ; - - // Check if refCoefRangeName is identical to default range for all observables, - // If so, substitute by unit integral - - // ---------- - RooArgSet tmpObs; - pdf->getObservables(&refCoefNorm, tmpObs); - bool allIdent = true; - for (auto *rvarg : dynamic_range_cast(tmpObs)) { - if (rvarg) { - if (rvarg->getMin(RooNameReg::str(refCoefRangeName)) != rvarg->getMin() || - rvarg->getMax(RooNameReg::str(refCoefRangeName)) != rvarg->getMax()) { - allIdent = false; - } - } - } - // ------------- - - if (refCoefRangeName && !refCoefNorm.empty() && !allIdent) { - - RooArgSet tmp; - pdf->getObservables(&refCoefNorm, tmp); - rangeProj1 = std::unique_ptr{pdf->createIntegral(tmp, tmp, RooNameReg::str(refCoefRangeName))}; - - // rangeProj1->setOperMode(operMode()) ; - oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName() - << ") R1 = " << rangeProj1->GetName() << std::endl; - } - _refRangeProjList.emplace_back(std::move(rangeProj1)); - // Calculate range adjusted projection integral std::unique_ptr rangeProj2; - oocxcoutD(&addPdf, Caching) << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName() - << ") rangename = " << (rangeName ? rangeName : "") - << " nset = " << (nset ? *nset : RooArgSet()) << std::endl; - if (rangeName && !refCoefNorm.empty()) { - - rangeProj2 = std::unique_ptr{pdf->createIntegral(refCoefNorm, refCoefNorm, rangeName)}; - // rangeProj2->setOperMode(operMode()) ; - - } else if (addPdf.normRange()) { - + if (normRange != refCoefNormRange) { RooArgSet tmp; - pdf->getObservables(&refCoefNorm, tmp); - rangeProj2 = std::unique_ptr{pdf->createIntegral(tmp, tmp, addPdf.normRange())}; - oocxcoutD(&addPdf, Caching) << " " << addPdf.ClassName() << "::syncCoefProjList(" << addPdf.GetName() - << ") R2 = " << rangeProj2->GetName() << std::endl; + pdf->getObservables(refCoefNormSet.empty() ? nset : &refCoefNormSet, tmp); + auto int1 = std::unique_ptr{pdf->createIntegral(tmp, tmp, normRange.c_str())}; + auto int2 = std::unique_ptr{pdf->createIntegral(tmp, tmp, refCoefNormRange.c_str())}; + rangeProj2 = std::make_unique("rangeProj", "rangeProj", *int1, *int2); + rangeProj2->addOwnedComponents(std::move(int1), std::move(int2)); } + _rangeProjList.emplace_back(std::move(rangeProj2)); } } } +void AddCacheElem::print() const +{ + auto printVector = [](auto const &vec, const char *name) { + std::cout << "+++ " << name << ":" << std::endl; + for (auto const &arg : vec) { + std::cout << " "; + if (arg) + arg->Print(); + else + std::cout << "nullptr" << std::endl; + } + }; + + printVector(_suppNormList, "_suppNormList"); + printVector(_projList, "_projList"); + printVector(_suppProjList, "_suppProjList"); + printVector(_rangeProjList, "_rangeProjList"); +} + //////////////////////////////////////////////////////////////////////////////// /// List all RooAbsArg derived contents in this cache element @@ -221,10 +201,6 @@ RooArgList AddCacheElem::containedArgs(Action) if (arg) allNodes.add(*arg); } - for (auto const &arg : _refRangeProjList) { - if (arg) - allNodes.add(*arg); - } for (auto const &arg : _rangeProjList) { if (arg) allNodes.add(*arg); @@ -244,7 +220,7 @@ RooArgList AddCacheElem::containedArgs(Action) void RooAddHelpers::updateCoefficients(RooAbsPdf const &addPdf, std::vector &coefCache, RooArgList const &pdfList, bool haveLastCoef, AddCacheElem &cache, - const RooArgSet *nset, bool projectCoefs, RooArgSet const &refCoefNorm, + const RooArgSet *nset, bool projectCoefs, RooArgSet const &refCoefNormSet, bool allExtendable, int &coefErrCount) { // Straight coefficients @@ -255,7 +231,7 @@ void RooAddHelpers::updateCoefficients(RooAbsPdf const &addPdf, std::vector(arg); - coefCache[i] = pdf->expectedEvents(!refCoefNorm.empty() ? &refCoefNorm : nset); + coefCache[i] = pdf->expectedEvents(!refCoefNormSet.empty() ? &refCoefNormSet : nset); coefSum += coefCache[i]; i++; } diff --git a/roofit/roofitcore/src/RooAddHelpers.h b/roofit/roofitcore/src/RooAddHelpers.h index 20e1294e02fed..9949153858198 100644 --- a/roofit/roofitcore/src/RooAddHelpers.h +++ b/roofit/roofitcore/src/RooAddHelpers.h @@ -21,8 +21,8 @@ class RooArgSet; class AddCacheElem : public RooAbsCacheElement { public: AddCacheElem(RooAbsPdf const &addPdf, RooArgList const &pdfList, RooArgList const &coefList, const RooArgSet *nset, - const RooArgSet *iset, const char *rangeName, bool projectCoefs, RooArgSet const &refCoefNorm, - TNamed const *refCoefRangeName, int verboseEval); + const RooArgSet *iset, bool projectCoefs, RooArgSet const &refCoefNormSet, + std::string const &refCoefNormRange, int verboseEval); RooArgList containedArgs(Action) override; @@ -37,33 +37,27 @@ class AddCacheElem : public RooAbsCacheElement { return _suppProjList[idx] ? _suppProjList[idx]->getVal() : 1.0; } - inline double rangeProjScaleFactor(std::size_t idx) const { return rangeProjVal(idx) / refRangeProjVal(idx); } - -private: - inline double refRangeProjVal(std::size_t idx) const - { - return _refRangeProjList[idx] ? _refRangeProjList[idx]->getVal() : 1.0; - } - - inline double rangeProjVal(std::size_t idx) const + inline double rangeProjScaleFactor(std::size_t idx) const { return _rangeProjList[idx] ? _rangeProjList[idx]->getVal() : 1.0; } + void print() const; + +private: using OwningArgVector = std::vector>; OwningArgVector _suppNormList; ///< Supplemental normalization list OwningArgVector _projList; ///< Projection integrals to be multiplied with coefficients OwningArgVector _suppProjList; ///< Projection integrals to multiply with coefficients for supplemental normalization - OwningArgVector _refRangeProjList; ///< Range integrals to be multiplied with coefficients (reference range) - OwningArgVector _rangeProjList; ///< Range integrals to be multiplied with coefficients (target range) + OwningArgVector _rangeProjList; ///< Range integrals to be multiplied with coefficients (reference to target range) }; class RooAddHelpers { public: static void updateCoefficients(RooAbsPdf const &addPdf, std::vector &coefCache, RooArgList const &pdfList, bool haveLastCoef, AddCacheElem &cache, const RooArgSet *nset, bool projectCoefs, - RooArgSet const &refCoefNorm, bool allExtendable, int &coefErrCount); + RooArgSet const &refCoefNormSet, bool allExtendable, int &coefErrCount); }; #endif diff --git a/roofit/roofitcore/src/RooAddModel.cxx b/roofit/roofitcore/src/RooAddModel.cxx index 9734c7bd2a649..2650877eee321 100644 --- a/roofit/roofitcore/src/RooAddModel.cxx +++ b/roofit/roofitcore/src/RooAddModel.cxx @@ -311,19 +311,21 @@ Int_t RooAddModel::basisCode(const char* name) const /// integrals to calculated transformed fraction coefficients when a frozen reference frame is provided /// and projection integrals for similar transformations when a frozen reference range is provided. -AddCacheElem* RooAddModel::getProjCache(const RooArgSet* nset, const RooArgSet* iset, const char* rangeName) const +AddCacheElem* RooAddModel::getProjCache(const RooArgSet* nset, const RooArgSet* iset) const { // Check if cache already exists - auto cache = static_cast(_projCacheMgr.getObj(nset,iset,0,rangeName)); + auto cache = static_cast(_projCacheMgr.getObj(nset,iset,0,normRange())); if (cache) { return cache ; } //Create new cache - cache = new AddCacheElem{*this, _pdfList, _coefList, nset, iset, rangeName, - _projectCoefs, _refCoefNorm, _refCoefRangeName, _verboseEval}; + cache = new AddCacheElem{*this, _pdfList, _coefList, nset, iset, + _projectCoefs, _refCoefNorm, + _refCoefRangeName ? RooNameReg::str(_refCoefRangeName) : "", + _verboseEval}; - _projCacheMgr.setObj(nset,iset,cache,RooNameReg::ptr(rangeName)) ; + _projCacheMgr.setObj(nset,iset,cache,RooNameReg::ptr(normRange())) ; return cache; } diff --git a/roofit/roofitcore/src/RooAddPdf.cxx b/roofit/roofitcore/src/RooAddPdf.cxx index aaaf63b872860..ea37f9e80b329 100644 --- a/roofit/roofitcore/src/RooAddPdf.cxx +++ b/roofit/roofitcore/src/RooAddPdf.cxx @@ -356,19 +356,21 @@ void RooAddPdf::fixCoefRange(const char* rangeName) /// - Projection integrals to calculate transformed fraction coefficients when a frozen reference frame is provided /// - Projection integrals for similar transformations when a frozen reference range is provided. -AddCacheElem* RooAddPdf::getProjCache(const RooArgSet* nset, const RooArgSet* iset, const char* rangeName) const +AddCacheElem* RooAddPdf::getProjCache(const RooArgSet* nset, const RooArgSet* iset) const { // Check if cache already exists - auto cache = static_cast(_projCacheMgr.getObj(nset,iset,0,rangeName)); + auto cache = static_cast(_projCacheMgr.getObj(nset,iset,0,normRange())); if (cache) { return cache ; } //Create new cache - cache = new AddCacheElem{*this, _pdfList, _coefList, nset, iset, rangeName, - _projectCoefs, _refCoefNorm, _refCoefRangeName, _verboseEval}; + cache = new AddCacheElem{*this, _pdfList, _coefList, nset, iset, + _projectCoefs, _refCoefNorm, + _refCoefRangeName ? RooNameReg::str(_refCoefRangeName) : "", + _verboseEval}; - _projCacheMgr.setObj(nset,iset,cache,RooNameReg::ptr(rangeName)) ; + _projCacheMgr.setObj(nset,iset,cache,RooNameReg::ptr(normRange())) ; return cache; } @@ -476,7 +478,7 @@ double RooAddPdf::getValV(const RooArgSet* normSet) const _value = 0.0; for (unsigned int i=0; i < _pdfList.size(); ++i) { - const auto& pdf = static_cast(_pdfList[i]); + auto& pdf = static_cast(_pdfList[i]); double snormVal = 1.; snormVal = cache->suppNormVal(i); @@ -658,7 +660,7 @@ double RooAddPdf::analyticalIntegralWN(Int_t code, const RooArgSet* normSet, con normSet = &_refCoefNorm ; } - AddCacheElem* cache = getProjCache(normSet,intSet,0) ; // WVE rangename here? + AddCacheElem* cache = getProjCache(normSet,intSet); updateCoefficients(*cache,normSet); // Calculate the current value of this object diff --git a/roofit/roofitcore/src/RooBinSamplingPdf.cxx b/roofit/roofitcore/src/RooBinSamplingPdf.cxx index 0abdbd0f29826..b1a3d677459a2 100644 --- a/roofit/roofitcore/src/RooBinSamplingPdf.cxx +++ b/roofit/roofitcore/src/RooBinSamplingPdf.cxx @@ -294,7 +294,7 @@ std::unique_ptr& RooBinSamplingPdf::integrator() c /// Binding used by the integrator to evaluate the PDF. double RooBinSamplingPdf::operator()(double x) const { _observable->setVal(x); - return _pdf->getVal(); + return _pdf; } diff --git a/roofit/roofitcore/src/RooDataSet.cxx b/roofit/roofitcore/src/RooDataSet.cxx index b3763d4eaf4df..40c4e0b0fa8df 100644 --- a/roofit/roofitcore/src/RooDataSet.cxx +++ b/roofit/roofitcore/src/RooDataSet.cxx @@ -100,6 +100,7 @@ the new `RooAbsData::uniqueId()`. #include "RooCompositeDataStore.h" #include "RooSentinel.h" #include "RooTrace.h" +#include "RooHelpers.h" #include "ROOT/StringUtils.hxx" @@ -886,7 +887,28 @@ RooAbsData* RooDataSet::reduceEng(const RooArgSet& varSubset, const RooFormulaVa if (_wgtVar) { tmp.add(*_wgtVar) ; } + + if (!cutRange || strchr(cutRange,',')==0) { return new RooDataSet(GetName(), GetTitle(), this, tmp, cutVar, cutRange, nStart, nStop) ; + } else { + // Composite case: multiple ranges + auto tokens = ROOT::Split(cutRange, ","); + if (RooHelpers::checkIfRangesOverlap(tmp, *this, tokens)) { + std::stringstream errMsg; + errMsg << "Error in RooAbsData::reduce! The ranges " << cutRange << " are overlapping!"; + throw std::runtime_error(errMsg.str()); + } + RooDataSet * out = nullptr; + for (const auto& token : tokens) { + if(!out) { + out = new RooDataSet(GetName(), GetTitle(), this, tmp, cutVar, token.c_str(), nStart, nStop); + } else { + RooDataSet appendedData{GetName(), GetTitle(), this, tmp, cutVar, token.c_str(), nStart, nStop}; + out->append(appendedData); + } + } + return out; + } } diff --git a/roofit/roofitcore/src/RooHelpers.cxx b/roofit/roofitcore/src/RooHelpers.cxx index e0ec13a386d1c..ed7c0aa850708 100644 --- a/roofit/roofitcore/src/RooHelpers.cxx +++ b/roofit/roofitcore/src/RooHelpers.cxx @@ -181,41 +181,13 @@ std::pair getRangeOrBinningInterval(RooAbsArg const* arg, const /// Check if there is any overlap when a list of ranges is applied to a set of observables. -/// \param[in] pdf the PDF +/// \param[in] observables The observables to check for overlap /// \param[in] data RooAbsCollection with the observables to check for overlap. /// \param[in] rangeNames The names of the ranges. -/// \param[in] splitRange If `true`, each component of a RooSimultaneous will -/// be checked individually for overlaps, with the range -/// names in that component suffixed by `_`. -/// See the `SplitRange()` command argument of -/// RooAbsPdf::fitTo()` to understand where this is used. -bool checkIfRangesOverlap(RooAbsPdf const& pdf, +bool checkIfRangesOverlap(RooArgSet const& observables, RooAbsData const& data, - std::vector const& rangeNames, - bool splitRange) + std::vector const& rangeNames) { - // If the PDF is a RooSimultaneous and the `splitRange` option is set, we - // have to check each component PDF with a different set of rangeNames, each - // suffixed by the category name. - if(splitRange && dynamic_cast(&pdf)) { - auto const& simPdf = static_cast(pdf); - bool hasOverlap = false; - std::vector rangeNamesSplit; - for (const auto& catState : simPdf.indexCat()) { - const std::string& catName = catState.first; - for(std::string const& rangeName : rangeNames) { - rangeNamesSplit.emplace_back(rangeName + "_" + catName); - } - hasOverlap |= checkIfRangesOverlap(*simPdf.getPdf(catName.c_str()), data, rangeNamesSplit, false); - - rangeNamesSplit.clear(); - } - - return hasOverlap; - } - - auto observables = *pdf.getObservables(data); - auto getLimits = [&](RooAbsRealLValue const& rlv, const char* rangeName) { // RooDataHistCase diff --git a/roofit/roofitcore/src/RooNLLVarNew.cxx b/roofit/roofitcore/src/RooNLLVarNew.cxx index f65423c2bb33d..4784acb8345c6 100644 --- a/roofit/roofitcore/src/RooNLLVarNew.cxx +++ b/roofit/roofitcore/src/RooNLLVarNew.cxx @@ -51,13 +51,6 @@ constexpr const char *RooNLLVarNew::weightVarNameSumW2; namespace { -std::unique_ptr -createFractionInRange(RooAbsPdf const &pdf, RooArgSet const &observables, std::string const &rangeNames) -{ - return std::unique_ptr{ - pdf.createIntegral(observables, &observables, pdf.getIntegratorConfig(), rangeNames.c_str())}; -} - template double kahanSum(Input const &input) { @@ -79,10 +72,9 @@ RooArgSet getObservablesInPdf(RooAbsPdf const &pdf, RooArgSet const &observables \param pdf The pdf for which the nll is computed for \param observables The observabes of the pdf \param isExtended Set to true if this is an extended fit -\param rangeName the range name **/ RooNLLVarNew::RooNLLVarNew(const char *name, const char *title, RooAbsPdf &pdf, RooArgSet const &observables, - bool isExtended, std::string const &rangeName, bool doOffset) + bool isExtended, bool doOffset) : RooAbsReal(name, title), _pdf{"pdf", "pdf", this, pdf}, _observables{getObservablesInPdf(pdf, observables)}, _isExtended{isExtended}, _doOffset{doOffset}, _weightVar{"weightVar", "weightVar", this, *new RooRealVar(weightVarName, weightVarName, 1.0), true, false, true}, @@ -133,13 +125,6 @@ RooNLLVarNew::RooNLLVarNew(const char *name, const char *title, RooAbsPdf &pdf, } } - if (!rangeName.empty()) { - auto term = createFractionInRange(*actualPdf, _observables, rangeName); - _fractionInRange = - std::make_unique>("_fractionInRange", "_fractionInRange", this, *term); - addOwnedComponents(std::move(term)); - } - resetWeightVarNames(); } @@ -150,9 +135,6 @@ RooNLLVarNew::RooNLLVarNew(const RooNLLVarNew &other, const char *name) this, other._weightSquaredVar} { - if (other._fractionInRange) - _fractionInRange = - std::make_unique>("_fractionInRange", this, *other._fractionInRange); } /** Compute multiple negative logs of propabilities @@ -225,34 +207,12 @@ void RooNLLVarNew::computeBatch(cudaStream_t * /*stream*/, double *output, size_ _logProbasBuffer.resize(nEvents); (*_pdf).getLogProbabilities(probas, _logProbasBuffer.data()); - if ((_isExtended || _fractionInRange) && _sumWeight == 0.0) { + if (_isExtended && _sumWeight == 0.0) { _sumWeight = weights.size() == 1 ? weights[0] * nEvents : kahanSum(weights); } - if ((_isExtended || _fractionInRange) && _weightSquared && _sumWeight2 == 0.0) { + if (_isExtended && _weightSquared && _sumWeight2 == 0.0) { _sumWeight2 = weights.size() == 1 ? weightsSumW2[0] * nEvents : kahanSum(weightsSumW2); } - double sumCorrectionTerm = 0; - if (_fractionInRange) { - auto fractionInRangeSpan = dataMap.at(*_fractionInRange); - if (fractionInRangeSpan.size() == 1) { - sumCorrectionTerm = (_weightSquared ? _sumWeight2 : _sumWeight) * std::log(fractionInRangeSpan[0]); - } else { - if (weightSpan.size() == 1) { - double fractionInRangeLogSum = 0.0; - for (std::size_t i = 0; i < fractionInRangeSpan.size(); ++i) { - fractionInRangeLogSum += std::log(fractionInRangeSpan[i]); - } - sumCorrectionTerm = weightSpan[0] * fractionInRangeLogSum; - } else { - // We don't need to use the library for now because the weights and - // correction term integrals are always in the CPU map. - sumCorrectionTerm = 0.0; - for (std::size_t i = 0; i < nEvents; ++i) { - sumCorrectionTerm += weightSpan[i] * std::log(fractionInRangeSpan[i]); - } - } - } - } ROOT::Math::KahanSum kahanProb; RooNaNPacker packedNaN(0.f); @@ -277,14 +237,8 @@ void RooNLLVarNew::computeBatch(cudaStream_t * /*stream*/, double *output, size_ if (_isExtended) { assert(_sumWeight != 0.0); double expected = _pdf->expectedEvents(&_observables); - if (_fractionInRange) { - expected *= dataMap.at(*_fractionInRange)[0]; - } kahanProb += _pdf->extendedTerm(_sumWeight, expected, _weightSquared ? _sumWeight2 : 0.0); } - if (_fractionInRange) { - kahanProb += sumCorrectionTerm; - } // Check if value offset flag is set. if (_doOffset) { diff --git a/roofit/roofitcore/src/RooNormalizedPdf.h b/roofit/roofitcore/src/RooNormalizedPdf.h index f30ed5e8747ba..764d930b319dc 100644 --- a/roofit/roofitcore/src/RooNormalizedPdf.h +++ b/roofit/roofitcore/src/RooNormalizedPdf.h @@ -21,7 +21,7 @@ class RooNormalizedPdf : public RooAbsPdf { RooNormalizedPdf(RooAbsPdf &pdf, RooArgSet const &normSet) : _pdf("numerator", "numerator", this, pdf), _normIntegral("denominator", "denominator", this, - *pdf.createIntegral(normSet, *pdf.getIntegratorConfig(), nullptr), true, false, true), + *pdf.createIntegral(normSet, *pdf.getIntegratorConfig(), pdf.normRange()), true, false, true), _normSet{normSet} { auto name = std::string(pdf.GetName()) + "_over_" + _normIntegral->GetName(); @@ -59,7 +59,7 @@ class RooNormalizedPdf : public RooAbsPdf { } protected: - void computeBatch(cudaStream_t *, double *output, size_t size, RooFit::Detail::DataMap const&) const override; + void computeBatch(cudaStream_t *, double *output, size_t size, RooFit::Detail::DataMap const &) const override; double evaluate() const override { // Evaluate() should not be called in the BatchMode, but we still need it diff --git a/roofit/roofitcore/test/testRooAddPdf.cxx b/roofit/roofitcore/test/testRooAddPdf.cxx index f3501bd370fc6..b6221643e4e78 100644 --- a/roofit/roofitcore/test/testRooAddPdf.cxx +++ b/roofit/roofitcore/test/testRooAddPdf.cxx @@ -3,8 +3,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -179,3 +181,59 @@ TEST(RooAddPdf, RecursiveCoefficients) EXPECT_TRUE(curve2->isIdentical(*curve1)); } + +class ProjCacheTest : public testing::TestWithParam> { + void SetUp() override { _fixCoefNormalization = std::get<0>(GetParam()); } + + void TearDown() override {} + +protected: + bool _fixCoefNormalization = false; +}; + +/// Verify that the coefficient projection works for different configurations +/// of the RooAddPdf. +TEST_P(ProjCacheTest, Test) +{ + RooRealVar x{"x", "x", 0, 4}; + x.setBins(4); + x.setRange("B1", 0, 2); + x.setRange("B2", 2, 4); + x.setRange("FULL", 0, 4); + + RooDataHist dataHist1{"dh1", "dh1", x}; + RooDataHist dataHist2{"dh2", "dh2", x}; + + for (int i = 0; i < 4; ++i) { + dataHist1.set(i, i + 1, 0.0); + dataHist2.set(i, 4. - i, 0.0); + } + + RooHistPdf pdf1{"pdf1", "pdf1", x, dataHist1}; + RooHistPdf pdf2{"pdf2", "pdf2", x, dataHist2}; + + RooRealVar frac{"frac", "frac", 0.5, 0, 1}; + + RooAddPdf pdf{"pdf", "pdf", {pdf1, pdf2}, frac}; + + RooArgSet normSet{x}; + + x.setRange(2, 4); + pdf.fixCoefRange("FULL"); + if (_fixCoefNormalization) { + pdf.fixCoefNormalization(normSet); + } + + x.setVal(2.5); + EXPECT_FLOAT_EQ(pdf.getVal(normSet), 0.5); + x.setVal(3.5); + EXPECT_FLOAT_EQ(pdf.getVal(normSet), 0.5); +} + +INSTANTIATE_TEST_SUITE_P(RooAddPdf, ProjCacheTest, + testing::Values(ProjCacheTest::ParamType{false}, ProjCacheTest::ParamType{true}), + [](testing::TestParamInfo const ¶mInfo) { + std::stringstream ss; + ss << (std::get<0>(paramInfo.param) ? "fixCoefNorm" : "floatingCoefNorm"); + return ss.str(); + }); diff --git a/roofit/roofitcore/test/testRooBinSamplingPdf.cxx b/roofit/roofitcore/test/testRooBinSamplingPdf.cxx index 798052527e44c..0999a2cc727b3 100644 --- a/roofit/roofitcore/test/testRooBinSamplingPdf.cxx +++ b/roofit/roofitcore/test/testRooBinSamplingPdf.cxx @@ -95,7 +95,6 @@ TEST(RooBinSamplingPdf, CheckConsistentNormalization) // An integral over the normalization set normalized by an integral over the // normalization set should be unity by definition. std::unique_ptr int1{binSamplingPdf.createIntegral(normSet, &normSet)}; - std::cout << int1->getVal() << std::endl; EXPECT_FLOAT_EQ(int1->getVal(), 1.0); // Evaluating the pdf with a given normalization set should not unexpectedly @@ -103,7 +102,5 @@ TEST(RooBinSamplingPdf, CheckConsistentNormalization) std::unique_ptr int2{binSamplingPdf.createIntegral(normSet)}; binSamplingPdf.getVal(normSet); std::unique_ptr int3{binSamplingPdf.createIntegral(normSet)}; - std::cout << int2->getVal() << std::endl; - std::cout << int3->getVal() << std::endl; EXPECT_FLOAT_EQ(int2->getVal(), int3->getVal()); } diff --git a/roofit/roofitcore/test/testRooSimultaneous.cxx b/roofit/roofitcore/test/testRooSimultaneous.cxx index aad39051e2c67..e6194268e182d 100644 --- a/roofit/roofitcore/test/testRooSimultaneous.cxx +++ b/roofit/roofitcore/test/testRooSimultaneous.cxx @@ -1,6 +1,7 @@ // Tests for the RooSimultaneous // Authors: Jonas Rembser, CERN 06/2021 +#include #include #include #include @@ -140,7 +141,7 @@ TEST(RooSimultaneous, MultiRangeFitWithSplitRange) int nEventsInCat = 11000; RooWorkspace wsCat1{"wsCat1"}; - wsCat1.factory("Gaussian::pdf_cat1(x_cat1[0,10],muCat1[4,0,10],sigma_cat1[1.0,0.1,10.0])"); + wsCat1.factory("Gaussian::pdf_cat1(x_cat1[0,10],mu_cat1[4,0,10],sigma_cat1[1.0,0.1,10.0])"); RooAbsPdf &pdfCat1 = *wsCat1.pdf("pdf_cat1"); RooRealVar &xCat1 = *wsCat1.var("x_cat1"); xCat1.setRange("SideBandLo_cat1", 0, 2); @@ -148,12 +149,12 @@ TEST(RooSimultaneous, MultiRangeFitWithSplitRange) std::unique_ptr dsCat1{pdfCat1.generate(xCat1, nEventsInCat)}; RooWorkspace wsCat2{"wsCat2"}; - wsCat2.factory("Gaussian::pdf_cat2(x_cat2[0,10],muCat2[6,0,10],sigma_cat2[1.0,0.1,10.0])"); + wsCat2.factory("Gaussian::pdf_cat2(x_cat2[0,10],mu_cat2[6,0,10],sigma_cat2[1.0,0.1,10.0])"); RooAbsPdf &pdfCat2 = *wsCat2.pdf("pdf_cat2"); RooRealVar &xCat2 = *wsCat2.var("x_cat2"); xCat2.setRange("SideBandLo_cat2", 0, 4); xCat2.setRange("SideBandHi_cat2", 8, 10); - std::unique_ptr dsCat2{pdfCat1.generate(xCat2, nEventsInCat)}; + std::unique_ptr dsCat2{pdfCat2.generate(xCat2, nEventsInCat)}; RooCategory indexCat{"cat", "cat"}; indexCat.defineType("cat1"); @@ -167,33 +168,24 @@ TEST(RooSimultaneous, MultiRangeFitWithSplitRange) RooDataSet combData{"combData", "", {xCat1, xCat2}, Index(indexCat), Import(dsmap)}; - std::unique_ptr nll1{pdfCat1.createNLL(*dsCat1, Range("SideBandLo_cat1,SideBandHi_cat1"))}; - std::unique_ptr nll2{pdfCat2.createNLL(*dsCat2, Range("SideBandLo_cat2,SideBandHi_cat2"))}; + const char *cutRange1 = "SideBandLo_cat1,SideBandHi_cat1"; + const char *cutRange2 = "SideBandLo_cat2,SideBandHi_cat2"; std::unique_ptr nllSim{simPdf.createNLL(combData, Range("SideBandLo,SideBandHi"), SplitRange())}; - // For now, we can't cross check the likelihood value and the unit test - // only checks whether the simultaneous likelihood can be created with the - // SplitRange() argument without the range overlap checks failing. Once the - // issue with multi-range simultaneous likelihood values is fixed, the rest - // of the test can be commented out: - - //const double nll1Val = nll1->getVal(); - //const double nll2Val = nll2->getVal(); // In simultaneous PDFs, the probability is normalized over the categories, // so we have to do that as well when computing the reference value. Since // we do a ranged fit, we have to consider the ranges when calculating the // number of events in data. - //double nCat1InRange = std::unique_ptr - //{ - //dsCat1->reduce(CutRange("SideBandLo_cat1,SideBandHi_cat1")) - //} -> sumEntries(); - //double nCat2InRange = std::unique_ptr - //{ - //dsCat2->reduce(CutRange("SideBandLo_cat2,SideBandHi_cat2")) - //} -> sumEntries(); - //const double nllSimRefVal = (nCat1InRange + nCat2InRange) * std::log(2) + nll1Val + nll2Val; - - //const double nllSimVal = nllSim->getVal(); - - //EXPECT_FLOAT_EQ(nllSimVal, nllSimRefVal); + double n1 = std::unique_ptr(dsCat1->reduce(CutRange(cutRange1)))->sumEntries(); + double n2 = std::unique_ptr(dsCat2->reduce(CutRange(cutRange2)))->sumEntries(); + const double normTerm = (n1 + n2) * std::log(2); + + std::unique_ptr nll1{pdfCat1.createNLL(*dsCat1, Range(cutRange1))}; + std::unique_ptr nll2{pdfCat2.createNLL(*dsCat2, Range(cutRange2))}; + RooAddition nllSimRef{"nllSimRef", "nllSimRef", {*nll1, *nll2, RooConst(normTerm)}}; + + const double nllSimRefVal = nllSimRef.getVal(); + const double nllSimVal = nllSim->getVal(); + + EXPECT_FLOAT_EQ(nllSimVal, nllSimRefVal); } diff --git a/roofit/roofitcore/test/testTestStatistics.cxx b/roofit/roofitcore/test/testTestStatistics.cxx index d63bb405ef066..774d2fbf5ed15 100644 --- a/roofit/roofitcore/test/testTestStatistics.cxx +++ b/roofit/roofitcore/test/testTestStatistics.cxx @@ -18,6 +18,8 @@ #include +const bool batchMode = true; + TEST(RooNLLVar, IntegrateBins) { RooRandom::randomGenerator()->SetSeed(1337ul); @@ -48,7 +50,7 @@ TEST(RooNLLVar, IntegrateBins) { a.setVal(3.); std::unique_ptr fit2( pdf.fitTo(data, RooFit::Save(), RooFit::PrintLevel(-1), - RooFit::BatchMode(true), + RooFit::BatchMode(batchMode), RooFit::IntegrateBins(1.E-3)) ); pdf.plotOn(frame.get(), RooFit::LineColor(kBlue), RooFit::Name("highRes")); @@ -107,14 +109,14 @@ TEST(RooNLLVar, IntegrateBins_SubRange) { std::unique_ptr fit1( pdf.fitTo(data, RooFit::Save(), RooFit::PrintLevel(-1), RooFit::Optimize(0), RooFit::Range("range"), - RooFit::BatchMode(true)) ); + RooFit::BatchMode(batchMode)) ); pdf.plotOn(frame.get(), RooFit::LineColor(kRed), RooFit::Name("standard")); a.setVal(3.); std::unique_ptr fit2( pdf.fitTo(data, RooFit::Save(), RooFit::PrintLevel(-1), RooFit::Optimize(0), RooFit::Range("range"), - RooFit::BatchMode(true), + RooFit::BatchMode(batchMode), RooFit::IntegrateBins(1.E-3)) ); pdf.plotOn(frame.get(), RooFit::LineColor(kBlue), RooFit::Name("highRes")); @@ -179,7 +181,7 @@ TEST(RooNLLVar, IntegrateBins_CustomBinning) { a.setVal(3.); std::unique_ptr fit2( pdf.fitTo(data, RooFit::Save(), RooFit::PrintLevel(-1), RooFit::Optimize(0), - RooFit::BatchMode(true), + RooFit::BatchMode(batchMode), RooFit::IntegrateBins(1.E-3)) ); pdf.plotOn(frame.get(), RooFit::LineColor(kBlue), RooFit::Name("highRes")); @@ -226,14 +228,14 @@ TEST(RooNLLVar, IntegrateBins_RooDataHist) { a.setVal(3.); std::unique_ptr fit1( pdf.fitTo(*data, RooFit::Save(), RooFit::PrintLevel(-1), - RooFit::BatchMode(true), + RooFit::BatchMode(batchMode), RooFit::IntegrateBins(-1.) // Disable forcefully ) ); pdf.plotOn(frame.get(), RooFit::LineColor(kRed), RooFit::Name("standard")); a.setVal(3.); std::unique_ptr fit2( pdf.fitTo(*data, RooFit::Save(), RooFit::PrintLevel(-1), - RooFit::BatchMode(true), + RooFit::BatchMode(batchMode), RooFit::IntegrateBins(0.) // Auto-enable for all RooDataHists. ) ); pdf.plotOn(frame.get(), RooFit::LineColor(kBlue), RooFit::Name("highRes")); @@ -283,7 +285,7 @@ TEST(RooChi2Var, IntegrateBins) { a.setVal(3.); std::unique_ptr fit2( pdf.chi2FitTo(*dataH, RooFit::Save(), RooFit::PrintLevel(-1), - RooFit::BatchMode(true), + RooFit::BatchMode(batchMode), RooFit::IntegrateBins(1.E-3)) ); pdf.plotOn(frame.get(), RooFit::LineColor(kBlue), RooFit::Name("highRes"));