diff --git a/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx b/roofit/roofitcore/src/RooAbsOptTestStatistic.cxx index 0246c3e3f086c5..f2d63cb7e4205b 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 855ac6f1fdbb12..2a68a7d3b11e51 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 145016755a7a55..7f3522058e3a33 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;