Skip to content

[RF] Support RooAbsCategoryLValue::setBin() for non-existing ranges #12305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 14, 2023
Merged

[RF] Support RooAbsCategoryLValue::setBin() for non-existing ranges #12305

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a05f2d5
[RF] Support `RooAbsCategoryLValue::setBin()` for non-existing ranges
guitargeek Feb 13, 2023
26ed0c7
[RF] Some code modernization is category classes
guitargeek Feb 13, 2023
4f5d844
[RF] Less manual memory management with `new` and `delete`
guitargeek Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions roofit/histfactory/src/ParamHistFunc.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ std::list<double>* ParamHistFunc::plotSamplingHint(RooAbsRealLValue& obs, double
RooAbsLValue* lvarg = &obs;

// Retrieve position of all bin boundaries
const RooAbsBinning* binning = lvarg->getBinningPtr(0) ;
const RooAbsBinning* binning = lvarg->getBinningPtr(nullptr);
double* boundaries = binning->array() ;

std::list<double>* hint = new std::list<double> ;
Expand Down Expand Up @@ -735,7 +735,7 @@ std::list<double>* ParamHistFunc::binBoundaries(RooAbsRealLValue& obs, double xl
RooAbsLValue* lvarg = &obs;

// Retrieve position of all bin boundaries
const RooAbsBinning* binning = lvarg->getBinningPtr(0) ;
const RooAbsBinning* binning = lvarg->getBinningPtr(nullptr);
double* boundaries = binning->array() ;

std::list<double>* hint = new std::list<double> ;
Expand Down
6 changes: 3 additions & 3 deletions roofit/roofit/inc/RooIntegralMorph.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ class RooIntegralMorph : public RooAbsCachedPdf {
void interpolateGap(Int_t ixlo, Int_t ixhi) ;

RooIntegralMorph* _self ; //
RooArgSet* _nset ;
std::unique_ptr<RooArgSet> _nset ;
RooAbsPdf* _pdf1 ; // PDF1
RooAbsPdf* _pdf2 ; // PDF2
RooRealVar* _x ; // X
Expand All @@ -77,8 +77,8 @@ class RooIntegralMorph : public RooAbsCachedPdf {
RooAbsReal* _c2 ; // CDF of PDF 2
RooAbsFunc* _cb1 ; // Binding of CDF1
RooAbsFunc* _cb2 ; // Binding of CDF2
RooBrentRootFinder* _rf1 ; // ROOT finder on CDF1
RooBrentRootFinder* _rf2 ; // ROOT finder of CDF2 ;
std::unique_ptr<RooBrentRootFinder> _rf1; // ROOT finder on CDF1
std::unique_ptr<RooBrentRootFinder> _rf2; // ROOT finder of CDF2 ;

std::vector<double> _yatX ; //
std::vector<double> _calcX; //
Expand Down
43 changes: 19 additions & 24 deletions roofit/roofit/src/RooIntegralMorph.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,14 +149,14 @@ RooArgSet* RooIntegralMorph::actualObservables(const RooArgSet& /*nset*/) const

RooArgSet* RooIntegralMorph::actualParameters(const RooArgSet& /*nset*/) const
{
RooArgSet* par1 = pdf1.arg().getParameters(RooArgSet()) ;
RooArgSet* par2 = pdf2.arg().getParameters(RooArgSet()) ;
par1->add(*par2,true) ;
RooArgSet* par1 = pdf1.arg().getParameters(static_cast<RooArgSet*>(nullptr));
RooArgSet par2;
pdf2.arg().getParameters(nullptr, par2);
par1->add(par2,true) ;
par1->remove(x.arg(),true,true) ;
if (!_cacheAlpha) {
par1->add(alpha.arg()) ;
}
delete par2 ;
return par1 ;
}

Expand Down Expand Up @@ -185,26 +185,23 @@ void RooIntegralMorph::fillCacheObject(PdfCacheElem& cache) const

if (!_cacheAlpha) {

TIterator* dIter = cache.hist()->sliceIterator((RooAbsArg&)x.arg(),RooArgSet()) ;
mcache.calculate(dIter) ;
delete dIter ;
std::unique_ptr<TIterator> dIter{cache.hist()->sliceIterator(const_cast<RooAbsReal&>(x.arg()),RooArgSet())};
mcache.calculate(dIter.get());

} else {
TIterator* slIter = cache.hist()->sliceIterator((RooAbsArg&)alpha.arg(),RooArgSet()) ;
std::unique_ptr<TIterator> slIter{cache.hist()->sliceIterator(const_cast<RooAbsReal&>(alpha.arg()),RooArgSet())};

double alphaSave = alpha ;
RooArgSet alphaSet(alpha.arg()) ;
coutP(Eval) << "RooIntegralMorph::fillCacheObject(" << GetName() << ") filling multi-dimensional cache" ;
while(slIter->Next()) {
alphaSet.assign(*cache.hist()->get()) ;
TIterator* dIter = cache.hist()->sliceIterator((RooAbsArg&)x.arg(),RooArgSet(alpha.arg())) ;
mcache.calculate(dIter) ;
std::unique_ptr<TIterator> dIter{cache.hist()->sliceIterator(const_cast<RooAbsReal&>(x.arg()),RooArgSet(alpha.arg()))};
mcache.calculate(dIter.get());
ccoutP(Eval) << "." << flush;
delete dIter ;
}
ccoutP(Eval) << endl ;
ccoutP(Eval) << std::endl;

delete slIter ;
const_cast<RooIntegralMorph*>(this)->alpha = alphaSave ;
}
}
Expand Down Expand Up @@ -244,19 +241,19 @@ RooIntegralMorph::MorphCacheElem::MorphCacheElem(RooIntegralMorph& self, const R
{
// Mark in base class that normalization of cached pdf is invariant under pdf parameters
_x = (RooRealVar*)self.x.absArg() ;
_nset = new RooArgSet(*_x) ;
_nset = std::make_unique<RooArgSet>(*_x);

_alpha = (RooAbsReal*)self.alpha.absArg() ;
_pdf1 = (RooAbsPdf*)(self.pdf1.absArg()) ;
_pdf2 = (RooAbsPdf*)(self.pdf2.absArg()) ;
_c1 = _pdf1->createCdf(*_x);
_c2 = _pdf2->createCdf(*_x) ;
_cb1 = _c1->bindVars(*_x,_nset) ;
_cb2 = _c2->bindVars(*_x,_nset) ;
_cb1 = _c1->bindVars(*_x,_nset.get());
_cb2 = _c2->bindVars(*_x,_nset.get());
_self = &self ;

_rf1 = new RooBrentRootFinder(*_cb1) ;
_rf2 = new RooBrentRootFinder(*_cb2) ;
_rf1 = std::make_unique<RooBrentRootFinder>(*_cb1);
_rf2 = std::make_unique<RooBrentRootFinder>(*_cb2);
_ccounter = 0 ;

_rf1->setTol(1e-12) ;
Expand All @@ -278,10 +275,6 @@ RooIntegralMorph::MorphCacheElem::MorphCacheElem(RooIntegralMorph& self, const R

RooIntegralMorph::MorphCacheElem::~MorphCacheElem()
{
delete _rf1 ;
delete _rf2 ;
// delete[] _yatX ;
// delete[] _calcX ;
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -421,8 +414,10 @@ void RooIntegralMorph::MorphCacheElem::calculate(TIterator* dIter)
_rf1->findRoot(x1,x1,xMax,y) ;
_rf2->findRoot(x2,x2,xMax,y) ;

_x->setVal(x1) ; double f1x1 = _pdf1->getVal(_nset) ;
_x->setVal(x2) ; double f2x2 = _pdf2->getVal(_nset) ;
_x->setVal(x1);
double f1x1 = _pdf1->getVal(_nset.get());
_x->setVal(x2);
double f2x2 = _pdf2->getVal(_nset.get());
double fbarX = f1x1*f2x2 / ( _alpha->getVal()*f2x2 + (1-_alpha->getVal())*f1x1 ) ;

dIter->Next() ;
Expand Down
4 changes: 2 additions & 2 deletions roofit/roofit/src/RooParamHistFunc.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ list<double>* RooParamHistFunc::plotSamplingHint(RooAbsRealLValue& obs, double x
}

// Retrieve position of all bin boundaries
const RooAbsBinning* binning = lvarg->getBinningPtr(0) ;
const RooAbsBinning* binning = lvarg->getBinningPtr(nullptr);
double* boundaries = binning->array() ;

list<double>* hint = new list<double> ;
Expand Down Expand Up @@ -208,7 +208,7 @@ std::list<double>* RooParamHistFunc::binBoundaries(RooAbsRealLValue& obs, double
}

// Retrieve position of all bin boundaries
const RooAbsBinning* binning = lvarg->getBinningPtr(0) ;
const RooAbsBinning* binning = lvarg->getBinningPtr(nullptr);
double* boundaries = binning->array() ;

list<double>* hint = new list<double> ;
Expand Down
9 changes: 5 additions & 4 deletions roofit/roofitcore/inc/RooAbsCategoryLValue.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
#ifndef ROO_ABS_CATEGORY_LVALUE
#define ROO_ABS_CATEGORY_LVALUE

#include "RooAbsCategory.h"
#include "RooAbsLValue.h"
#include <RooAbsCategory.h>
#include <RooAbsLValue.h>

#include <list>
#include <string>
#include <utility>
#include <utility> // for std::pair

class RooAbsCategoryLValue : public RooAbsCategory, public RooAbsLValue {
public:
Expand All @@ -30,7 +31,7 @@ class RooAbsCategoryLValue : public RooAbsCategory, public RooAbsLValue {
}
RooAbsCategoryLValue(const char *name, const char *title);
RooAbsCategoryLValue(const RooAbsCategoryLValue& other, const char* name=nullptr) ;
~RooAbsCategoryLValue() override;
~RooAbsCategoryLValue() override = default;

// Value modifiers
////////////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions roofit/roofitcore/src/RooAbsAnaConvPdf.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,11 +335,11 @@ double RooAbsAnaConvPdf::evaluate() const
auto conv = static_cast<RooAbsPdf*>(convArg);
double coef = coefficient(index++) ;
if (coef!=0.) {
double c = conv->getVal(0) ;
double c = conv->getVal(nullptr);
double r = coef ;
cxcoutD(Eval) << "RooAbsAnaConvPdf::evaluate(" << GetName() << ") val += coef*conv [" << index-1 << "/"
<< _convSet.getSize() << "] coef = " << r << " conv = " << c << endl ;
result += conv->getVal(0)*coef ;
result += conv->getVal(nullptr)*coef ;
} else {
cxcoutD(Eval) << "RooAbsAnaConvPdf::evaluate(" << GetName() << ") [" << index-1 << "/" << _convSet.getSize() << "] coef = 0" << endl ;
}
Expand Down
67 changes: 29 additions & 38 deletions roofit/roofitcore/src/RooAbsCategory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ the following replacements should be used:
#include <functional>
#include <memory>

using namespace std;

ClassImp(RooAbsCategory);

/// A category state to signify an invalid category. The category name is empty,
Expand Down Expand Up @@ -247,13 +245,13 @@ const std::map<std::string, RooAbsCategory::value_type>::value_type& RooAbsCateg

if (hasIndex(index)) {
coutE(InputArguments) << "RooAbsCategory::" << __func__ << "(" << GetName() << "): index "
<< index << " already assigned" << endl ;
<< index << " already assigned" << std::endl;
return invalidCategory();
}

if (hasLabel(label)) {
coutE(InputArguments) << "RooAbsCategory::" << __func__ << "(" << GetName() << "): label "
<< label << " already assigned or not allowed" << endl ;
<< label << " already assigned or not allowed" << std::endl;
return invalidCategory();
}

Expand Down Expand Up @@ -319,7 +317,7 @@ const RooCatType* RooAbsCategory::lookupType(RooAbsCategory::value_type index, b

if (printError) {
coutE(InputArguments) << ClassName() << "::" << GetName() << ":lookupType: no match for index "
<< index << endl;
<< index << std::endl;
}

return nullptr;
Expand Down Expand Up @@ -347,7 +345,7 @@ const RooCatType* RooAbsCategory::lookupType(const char* label, bool printError)

if (printError) {
coutE(InputArguments) << ClassName() << "::" << GetName() << ":lookupType: no match for label "
<< label << endl;
<< label << std::endl;
}
return nullptr;
}
Expand Down Expand Up @@ -376,7 +374,7 @@ Roo1DTable* RooAbsCategory::createTable(const char *label) const
////////////////////////////////////////////////////////////////////////////////
/// Read object contents from stream (dummy for now)

bool RooAbsCategory::readFromStream(istream&, bool, bool)
bool RooAbsCategory::readFromStream(std::istream&, bool, bool)
{
return false ;
}
Expand All @@ -386,7 +384,7 @@ bool RooAbsCategory::readFromStream(istream&, bool, bool)
////////////////////////////////////////////////////////////////////////////////
/// Write object contents to ostream

void RooAbsCategory::writeToStream(ostream& os, bool /* compact */) const
void RooAbsCategory::writeToStream(std::ostream& os, bool /* compact */) const
{
os << getCurrentLabel() ;
}
Expand All @@ -396,9 +394,9 @@ void RooAbsCategory::writeToStream(ostream& os, bool /* compact */) const
////////////////////////////////////////////////////////////////////////////////
/// Print value (label name)

void RooAbsCategory::printValue(ostream& os) const
void RooAbsCategory::printValue(std::ostream& os) const
{
os << getCurrentLabel() << "(idx = " << getCurrentIndex() << ")" << endl ;
os << getCurrentLabel() << "(idx = " << getCurrentIndex() << ")" << std::endl;
}


Expand All @@ -409,17 +407,17 @@ void RooAbsCategory::printValue(ostream& os) const
///
/// Shape : label, index, defined types

void RooAbsCategory::printMultiline(ostream& os, Int_t contents, bool verbose, TString indent) const
void RooAbsCategory::printMultiline(std::ostream& os, Int_t contents, bool verbose, TString indent) const
{
RooAbsArg::printMultiline(os,contents,verbose,indent);

os << indent << "--- RooAbsCategory ---" << endl;
os << indent << "--- RooAbsCategory ---" << std::endl;
if (stateNames().empty()) {
os << indent << " ** No values defined **" << endl;
os << indent << " ** No values defined **" << std::endl;
return;
}
os << indent << " Value = " << getCurrentIndex() << " \"" << getCurrentLabel() << ')' << endl;
os << indent << " Possible states:" << endl;
os << indent << " Value = " << getCurrentIndex() << " \"" << getCurrentLabel() << ')' << std::endl;
os << indent << " Possible states:" << std::endl;
indent.Append(" ");
for (const auto& type : stateNames()) {
os << indent << type.first << '\t' << type.second << "\n";
Expand Down Expand Up @@ -447,26 +445,26 @@ void RooAbsCategory::attachToVStore(RooVectorDataStore& vstore)
void RooAbsCategory::attachToTree(TTree& tree, Int_t bufSize)
{
// First check if there is an integer branch matching the category name
TString cleanName(cleanBranchName()) ;
TBranch* branch = tree.GetBranch(cleanName) ;
std::string cleanName = cleanBranchName().Data();
TBranch* branch = tree.GetBranch(cleanName.c_str());
if (!branch) {
cleanName += "_idx";
branch = tree.GetBranch(cleanName);
branch = tree.GetBranch(cleanName.c_str());
}

if (branch) {
TLeaf* leaf = (TLeaf*)branch->GetListOfLeaves()->At(0) ;
TLeaf* leaf = static_cast<TLeaf*>(branch->GetListOfLeaves()->At(0));

// Check that leaf is _not_ an array
Int_t dummy ;
TLeaf* counterLeaf = leaf->GetLeafCounter(dummy) ;
if (counterLeaf) {
coutE(Eval) << "RooAbsCategory::attachToTree(" << GetName() << ") ERROR: TTree branch " << GetName()
<< " is an array and cannot be attached to a RooAbsCategory" << endl ;
<< " is an array and cannot be attached to a RooAbsCategory" << std::endl;
return ;
}

TString typeName(leaf->GetTypeName()) ;
const std::string typeName = leaf->GetTypeName();


// For different type names, store a function to attach
Expand All @@ -483,26 +481,24 @@ void RooAbsCategory::attachToTree(TTree& tree, Int_t bufSize)
{"UShort_t", [&](){ return createTreeReadBuffer<UShort_t >(cleanName, tree); }},
};

auto typeDetails = typeMap.find(typeName.Data());
auto typeDetails = typeMap.find(typeName);
if (typeDetails != typeMap.end()) {
coutI(DataHandling) << "RooAbsCategory::attachToTree(" << GetName() << ") TTree " << typeName << " branch \"" << cleanName
<< "\" will be converted to int." << endl ;
<< "\" will be converted to int." << std::endl;
_treeReadBuffer = typeDetails->second();
} else {
_treeReadBuffer = nullptr;

if (!typeName.CompareTo("Int_t")) {
tree.SetBranchAddress(cleanName, &_currentIndex);
if (typeName == "Int_t") {
tree.SetBranchAddress(cleanName.c_str(), &_currentIndex);
}
else {
coutE(InputArguments) << "RooAbsCategory::attachToTree(" << GetName() << ") data type " << typeName << " is not supported." << endl ;
coutE(InputArguments) << "RooAbsCategory::attachToTree(" << GetName() << ") data type " << typeName << " is not supported." << std::endl;
}
}
} else {
TString format(cleanName);
format.Append("/I");
void* ptr = &_currentIndex;
tree.Branch(cleanName, ptr, (const Text_t*)format, bufSize);
tree.Branch(cleanName.c_str(), ptr, (cleanName + "/I").c_str(), bufSize);
}
}

Expand All @@ -513,13 +509,10 @@ void RooAbsCategory::attachToTree(TTree& tree, Int_t bufSize)

void RooAbsCategory::fillTreeBranch(TTree& t)
{
TString idxName(GetName()) ;
idxName.Append("_idx") ;

// First determine if branch is taken
TBranch* idxBranch = t.GetBranch(idxName) ;
TBranch* idxBranch = t.GetBranch((std::string(GetName()) + "_idx").c_str()) ;
if (!idxBranch) {
coutF(DataHandling) << "RooAbsCategory::fillTreeBranch(" << GetName() << ") ERROR: not attached to tree" << endl ;
coutF(DataHandling) << "RooAbsCategory::fillTreeBranch(" << GetName() << ") ERROR: not attached to tree" << std::endl;
throw std::runtime_error("RooAbsCategory::fillTreeBranch(): Category is not attached to a tree.");
}

Expand Down Expand Up @@ -699,7 +692,7 @@ RooCatType* RooAbsCategory::retrieveLegacyState(value_type index) const {
auto result = _legacyStates.find(index);
if (result == _legacyStates.end()) {
result = _legacyStates.emplace(index,
std::unique_ptr<RooCatType>(new RooCatType(lookupName(index).c_str(), index))).first;
std::make_unique<RooCatType>(lookupName(index).c_str(), index)).first;
}

return result->second.get();
Expand All @@ -713,7 +706,5 @@ RooAbsCategory::value_type RooAbsCategory::nextAvailableStateIndex() const {
return 0;

return 1 + std::max_element(theStateNames.begin(), theStateNames.end(),
[](const std::map<std::string, value_type>::value_type& left,
const std::map<std::string, value_type>::value_type& right) {
return left.second < right.second; })->second;
[](auto const& left, auto const& right) { return left.second < right.second; })->second;
}
Loading