Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class RooBarlowBeestonLL : public RooAbsReal {
};

void initializeBarlowCache();
bool getParameters(const RooArgSet* depList, RooArgSet& outputSet, bool stripDisconnected=true) const override;
void addParameters(RooAbsCollection & params, const RooArgSet *nset, RooFit::GetParametersPolicy const &policy) const override;
RooAbsReal& nll() { return const_cast<RooAbsReal&>(_nll.arg()) ; }
void setPdf(RooAbsPdf* pdf) { _pdf = pdf; }
void setDataset(RooAbsData* data) { _data = data; }
Expand Down
15 changes: 6 additions & 9 deletions roofit/histfactory/src/RooBarlowBeestonLL.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -298,15 +298,15 @@ void RooStats::HistFactory::RooBarlowBeestonLL::initializeBarlowCache() {

////////////////////////////////////////////////////////////////////////////////

bool RooStats::HistFactory::RooBarlowBeestonLL::getParameters(const RooArgSet* depList,
RooArgSet& outputSet,
bool stripDisconnected) const {
bool errorInBaseCall = RooAbsArg::getParameters( depList, outputSet, stripDisconnected );
void RooStats::HistFactory::RooBarlowBeestonLL::addParameters(RooAbsCollection &params, const RooArgSet *nset,
RooFit::GetParametersPolicy const &policy) const
{
RooAbsArg::addParameters(params, nset, policy);

RooArgSet toRemove;
toRemove.reserve( _statUncertParams.size());

for (auto const& arg : outputSet) {
for (auto const& arg : params) {

// If there is a gamma in the name,
// strip it from the list of dependencies
Expand All @@ -316,10 +316,7 @@ bool RooStats::HistFactory::RooBarlowBeestonLL::getParameters(const RooArgSet* d
}
}

for( auto& arg : toRemove) outputSet.remove( *arg, true );

return errorInBaseCall || false;

for( auto& arg : toRemove) params.remove( *arg, true );
}


Expand Down
40 changes: 29 additions & 11 deletions roofit/roofitcore/inc/RooAbsArg.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ namespace RooFit {
namespace Experimental {
class CodegenContext;
}

struct GetParametersPolicy {
bool stripDisconnected = false;
};

} // namespace RooFit

class RooRefArray : public TObjArray {
Expand Down Expand Up @@ -208,12 +213,27 @@ class RooAbsArg : public TNamed, public RooPrintable {
///////////////////////////////////////////////////////////////////////////////

// Parameter & observable interpretation of servers
RooFit::OwningPtr<RooArgSet> getVariables(bool stripDisconnected = true) const;
RooFit::OwningPtr<RooArgSet> getParameters(const RooAbsData *data, bool stripDisconnected = true) const;
RooFit::OwningPtr<RooArgSet> getParameters(const RooAbsData &data, bool stripDisconnected = true) const;
RooFit::OwningPtr<RooArgSet> getParameters(const RooArgSet &observables, bool stripDisconnected = true) const;
RooFit::OwningPtr<RooArgSet> getParameters(const RooArgSet *observables, bool stripDisconnected = true) const;
virtual bool getParameters(const RooArgSet *observables, RooArgSet &outputSet, bool stripDisconnected = true) const;
RooFit::OwningPtr<RooArgSet> getVariables() const;

auto getParameters(const RooArgSet &ref, RooFit::GetParametersPolicy const &policy = {}) const
{
return getParameters(&ref, policy);
}
auto getParameters(const RooAbsData &ref, RooFit::GetParametersPolicy const &policy = {}) const
{
return getParameters(&ref, policy);
}
auto getParameters(std::nullptr_t, RooFit::GetParametersPolicy const &policy = {}) const
{
return getParameters(static_cast<RooArgSet const *>(nullptr), policy);
}
RooFit::OwningPtr<RooArgSet>
getParameters(const RooAbsData *data, RooFit::GetParametersPolicy const &policy = {}) const;
RooFit::OwningPtr<RooArgSet>
getParameters(const RooArgSet *observables, RooFit::GetParametersPolicy const &policy = {}) const;
bool getParameters(const RooArgSet *observables, RooArgSet &outputSet,
RooFit::GetParametersPolicy const &policy = {}) const;

RooFit::OwningPtr<RooArgSet> getObservables(const RooArgSet &set, bool valueOnly = true) const;
RooFit::OwningPtr<RooArgSet> getObservables(const RooAbsData *data) const;
RooFit::OwningPtr<RooArgSet> getObservables(const RooAbsData &data) const;
Expand All @@ -225,6 +245,9 @@ class RooAbsArg : public TNamed, public RooPrintable {
bool recursiveCheckObservables(const RooArgSet *nset) const;
RooFit::OwningPtr<RooArgSet> getComponents() const;

virtual void
addParameters(RooAbsCollection &params, const RooArgSet *nset, RooFit::GetParametersPolicy const &policy) const;

void attachArgs(const RooAbsCollection &set);
void attachDataSet(const RooAbsData &set);
void attachDataStore(const RooAbsDataStore &set);
Expand Down Expand Up @@ -509,9 +532,6 @@ class RooAbsArg : public TNamed, public RooPrintable {

virtual bool isValid() const;

virtual void
getParametersHook(const RooArgSet * /*nset*/, RooArgSet * /*list*/, bool /*stripDisconnected*/) const {};

void clearValueAndShapeDirty() const
{
_valueDirty = false;
Expand All @@ -530,8 +550,6 @@ class RooAbsArg : public TNamed, public RooPrintable {
static void ioStreamerPass2Finalize();

private:
void addParameters(RooAbsCollection &params, const RooArgSet *nset = nullptr, bool stripDisconnected = true) const;

RefCountListLegacyIterator_t *makeLegacyIterator(const RefCountList_t &list) const;

protected:
Expand Down
5 changes: 3 additions & 2 deletions roofit/roofitcore/inc/RooEvaluatorWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,15 @@ class RooEvaluatorWrapper final : public RooAbsReal {

double defaultErrorLevel() const override { return _topNode->defaultErrorLevel(); }

bool getParameters(const RooArgSet *observables, RooArgSet &outputSet, bool stripDisconnected = true) const override;

bool setData(RooAbsData &data, bool cloneData) override;

double getValV(const RooArgSet *) const override { return evaluate(); }

void applyWeightSquared(bool flag) override { _topNode->applyWeightSquared(flag); }

void addParameters(RooAbsCollection &params, const RooArgSet *nset,
RooFit::GetParametersPolicy const &policy) const override;

bool hasGradient() const override;
bool hasHessian() const override;

Expand Down
3 changes: 2 additions & 1 deletion roofit/roofitcore/inc/RooMultiPdf.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class RooMultiPdf : public RooAbsPdf {
// Always normalized because each pdf is normalized
bool selfNormalized() const override { return true; }

void getParametersHook(const RooArgSet *nset, RooArgSet *list, bool stripDisconnected) const override;
void addParameters(RooAbsCollection &params, const RooArgSet *nset,
RooFit::GetParametersPolicy const &policy) const override;

protected:
RooListProxy c;
Expand Down
3 changes: 2 additions & 1 deletion roofit/roofitcore/inc/RooMultiReal.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ class RooMultiReal : public RooAbsReal {
inline const RooCategoryProxy &indexCategory() const { return _index; }
inline const RooListProxy &getModelList() const { return _models; }

void getParametersHook(const RooArgSet *nset, RooArgSet *list, bool stripDisconnected) const override;
void addParameters(RooAbsCollection &params, const RooArgSet *nset,
RooFit::GetParametersPolicy const &policy) const override;

protected:
RooListProxy _models; // list of RooAbsReal models
Expand Down
2 changes: 1 addition & 1 deletion roofit/roofitcore/src/ConstraintHelpers.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ std::unique_ptr<RooAbsReal> createConstraintTerm(std::string const &name, RooAbs
if (constrainedParameters) {
cPars.add(*constrainedParameters);
} else {
pdf.getParameters(&observables, cPars, false);
pdf.getParameters(&observables, cPars);
doStripDisconnected = true;
}

Expand Down
41 changes: 15 additions & 26 deletions roofit/roofitcore/src/RooAbsArg.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -505,21 +505,10 @@ void RooAbsArg::treeNodeServerList(RooAbsCollection *list, const RooAbsArg *arg,
/// function is responsible for deleting the returned argset.
/// The complement of this function is getObservables()

RooFit::OwningPtr<RooArgSet> RooAbsArg::getParameters(const RooAbsData *set, bool stripDisconnected) const
RooFit::OwningPtr<RooArgSet>
RooAbsArg::getParameters(const RooAbsData *set, RooFit::GetParametersPolicy const &policy) const
{
return getParameters(set ? set->get() : nullptr, stripDisconnected);
}

/// Return the parameters of this p.d.f when used in conjunction with dataset 'data'.
RooFit::OwningPtr<RooArgSet> RooAbsArg::getParameters(const RooAbsData &data, bool stripDisconnected) const
{
return getParameters(&data, stripDisconnected);
}

/// Return the parameters of the p.d.f given the provided set of observables.
RooFit::OwningPtr<RooArgSet> RooAbsArg::getParameters(const RooArgSet &observables, bool stripDisconnected) const
{
return getParameters(&observables, stripDisconnected);
return getParameters(set ? set->get() : nullptr, policy);
}

/// Create a list of leaf nodes in the arg tree starting with
Expand All @@ -528,19 +517,21 @@ RooFit::OwningPtr<RooArgSet> RooAbsArg::getParameters(const RooArgSet &observabl
/// for deleting the returned argset. The complement of this function
/// is getObservables().

RooFit::OwningPtr<RooArgSet> RooAbsArg::getParameters(const RooArgSet *observables, bool stripDisconnected) const
RooFit::OwningPtr<RooArgSet>
RooAbsArg::getParameters(const RooArgSet *observables, RooFit::GetParametersPolicy const &policy) const
{
auto *outputSet = new RooArgSet;
getParameters(observables, *outputSet, stripDisconnected);
getParameters(observables, *outputSet, policy);
return RooFit::OwningPtr<RooArgSet>{outputSet};
}

/// Add all parameters of the function and its daughters to `params`.
/// \param[in] params Collection that stores all parameters. Add all new parameters to this.
/// \param[in] nset Normalisation set (optional). If a value depends on this set, it's not a parameter.
/// \param[in] stripDisconnected Passed on to getParametersHook().
/// \param[in] stripDisconnected TODO.

void RooAbsArg::addParameters(RooAbsCollection &params, const RooArgSet *nset, bool stripDisconnected) const
void RooAbsArg::addParameters(RooAbsCollection &params, const RooArgSet *nset,
RooFit::GetParametersPolicy const &policy) const
{

RooArgSet nodeParamServers;
Expand All @@ -561,12 +552,9 @@ void RooAbsArg::addParameters(RooAbsCollection &params, const RooArgSet *nset, b
std::sort(branchList.begin(), branchList.end());
const auto last = std::unique(branchList.begin(), branchList.end());
for (auto serverIt = branchList.begin(); serverIt < last; ++serverIt) {
(*serverIt)->addParameters(nodeParamServers, nset, stripDisconnected);
(*serverIt)->addParameters(nodeParamServers, nset, policy);
}

// Allow pdf to strip parameters from list
getParametersHook(nset, &nodeParamServers, stripDisconnected);

// Add parameters of this node to the combined list
params.add(nodeParamServers, true);
}
Expand All @@ -579,12 +567,13 @@ void RooAbsArg::addParameters(RooAbsCollection &params, const RooArgSet *nset, b
/// \param[out] outputSet Output set.
/// \param[in] stripDisconnected Allow pdf to strip parameters from list before adding it.

bool RooAbsArg::getParameters(const RooArgSet *observables, RooArgSet &outputSet, bool stripDisconnected) const
bool RooAbsArg::getParameters(const RooArgSet *observables, RooArgSet &outputSet,
RooFit::GetParametersPolicy const &policy) const
{
outputSet.clear();
outputSet.setName("parameters");

addParameters(outputSet, observables, stripDisconnected);
addParameters(outputSet, observables, policy);

outputSet.sort();

Expand Down Expand Up @@ -1812,9 +1801,9 @@ RooAbsCache *RooAbsArg::getCache(Int_t index) const

/// Return RooArgSet with all variables (tree leaf nodes of expression tree)

RooFit::OwningPtr<RooArgSet> RooAbsArg::getVariables(bool stripDisconnected) const
RooFit::OwningPtr<RooArgSet> RooAbsArg::getVariables() const
{
return getParameters(RooArgSet(), stripDisconnected);
return getParameters(RooArgSet());
}

/// Create a GraphViz .dot file visualizing the expression tree headed by
Expand Down
7 changes: 4 additions & 3 deletions roofit/roofitcore/src/RooAbsData.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -1422,12 +1422,13 @@ SplittingSetup initSplit(RooAbsData const &data, RooAbsCategory const &splitCat)

// Construct set of variables to be included in split sets = full set - split category
setup.subsetVars.add(*data.get());
RooArgSet vars;
if (splitCat.isDerived()) {
std::unique_ptr<RooArgSet> vars{splitCat.getVariables()};
setup.subsetVars.remove(*vars, true, true);
splitCat.getParameters(nullptr, vars);
} else {
setup.subsetVars.remove(splitCat, true, true);
vars.add(splitCat);
}
setup.subsetVars.remove(vars, true, true);

// Add weight variable explicitly if dataset has weights, but no top-level weight
// variable exists (can happen with composite datastores)
Expand Down
12 changes: 7 additions & 5 deletions roofit/roofitcore/src/RooAbsReal.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -662,9 +662,10 @@ RooFit::OwningPtr<RooAbsReal> RooAbsReal::createIntObj(const RooArgSet& iset2, c
const char* cacheParamsStr = getStringAttribute("CACHEPARAMINT") ;
if (cacheParamsStr && strlen(cacheParamsStr)) {

std::unique_ptr<RooArgSet> intParams{integral->getVariables()};
RooArgSet intParams;
integral->getParameters(nullptr, intParams);

RooArgSet cacheParams = RooHelpers::selectFromArgSet(*intParams, cacheParamsStr);
RooArgSet cacheParams = RooHelpers::selectFromArgSet(intParams, cacheParamsStr);

if (!cacheParams.empty()) {
cxcoutD(Caching) << "RooAbsReal::createIntObj(" << GetName() << ") INFO: constructing " << cacheParams.size()
Expand Down Expand Up @@ -1176,7 +1177,8 @@ RooDataHist* RooAbsReal::fillDataHist(RooDataHist *hist, const RooArgSet* normSe

TH1* RooAbsReal::createHistogram(RooStringView varNameList, Int_t xbins, Int_t ybins, Int_t zbins) const
{
std::unique_ptr<RooArgSet> vars{getVariables()};
RooArgSet vars;
getParameters(nullptr, vars);

auto varNames = ROOT::Split(varNameList, ",:");
std::vector<RooRealVar*> histVars(3, nullptr);
Expand All @@ -1189,10 +1191,10 @@ TH1* RooAbsReal::createHistogram(RooStringView varNameList, Int_t xbins, Int_t y
coutE(Plotting) << errMsg.str() << std::endl;
throw std::invalid_argument(errMsg.str());
}
auto var = static_cast<RooRealVar*>(vars->find(varNames[iVar].c_str()));
auto var = static_cast<RooRealVar*>(vars.find(varNames[iVar].c_str()));
if(!var) {
std::stringstream errMsg;
errMsg << "RooAbsPdf::createHistogram(" << GetName() << ") ERROR variable " << varNames[iVar] << " does not exist in argset: " << *vars;
errMsg << "RooAbsPdf::createHistogram(" << GetName() << ") ERROR variable " << varNames[iVar] << " does not exist in argset: " << vars;
coutE(Plotting) << errMsg.str() << std::endl;
throw std::runtime_error(errMsg.str());
}
Expand Down
30 changes: 15 additions & 15 deletions roofit/roofitcore/src/RooEvaluatorWrapper.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -160,48 +160,48 @@

RooEvaluatorWrapper::~RooEvaluatorWrapper() = default;

bool RooEvaluatorWrapper::getParameters(const RooArgSet *observables, RooArgSet &outputSet,
bool stripDisconnected) const
void RooEvaluatorWrapper::addParameters(RooAbsCollection &params, const RooArgSet *nset,
RooFit::GetParametersPolicy const &policy) const
{
outputSet.add(_evaluator->getParameters());
if (observables) {
outputSet.remove(*observables, /*silent*/ false, /*matchByNameOnly*/ true);
params.add(_evaluator->getParameters());
if (nset) {
params.remove(*nset, /*silent*/ false, /*matchByNameOnly*/ true);
}
// Exclude the data variables from the parameters which are not global observables
for (auto const &item : _dataSpans) {
if (_data->getGlobalObservables() && _data->getGlobalObservables()->find(item.first->GetName())) {
continue;
}
RooAbsArg *found = outputSet.find(item.first->GetName());
RooAbsArg *found = params.find(item.first->GetName());
if (found) {
outputSet.remove(*found);
params.remove(*found);
}
}
// If we take the global observables as data, we have to return these as
// parameters instead of the parameters in the model. Otherwise, the
// constant parameters in the fit result that are global observables will
// not have the right values.
if (_takeGlobalObservablesFromData && _data->getGlobalObservables()) {
outputSet.replace(*_data->getGlobalObservables());
params.replace(*_data->getGlobalObservables());
}

// The disconnected parameters are stripped away in
// RooAbsArg::getParametersHook(), that is only called in the original
// RooAbsArg::getParameters() implementation. So he have to call it to
// identify disconnected parameters to remove.
if (stripDisconnected) {
if (policy.stripDisconnected) {
RooArgSet paramsStripped;
_topNode->getParameters(observables, paramsStripped, true);
RooFit::GetParametersPolicy policy;

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / alma9 CMAKE_BUILD_TYPE=Debug

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / alma10 arm64 CMAKE_BUILD_TYPE=RelWithDebInfo, builtin_zlib=ON, builtin_zstd=ON, CMAKE_CXX_STANDARD=20

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / alma10

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / ubuntu22 imt=Off, CMAKE_BUILD_TYPE=Debug

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / fedora44

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / ubuntu2404 CMAKE_BUILD_TYPE=Debug

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / ubuntu2604

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / fedora43

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / alma10 benchmark build CMAKE_CXX_STANDARD=20

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / alma8

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / alma9 modules_off CMAKE_CXX_STANDARD=20

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / opensuse16 march_native

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check warning on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / rawhide Fedora pydebug no GIL CMAKE_CXX_STANDARD=23

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Wshadow]

Check failure on line 194 in roofit/roofitcore/src/RooEvaluatorWrapper.cxx

View workflow job for this annotation

GitHub Actions / debian13 dev=ON, CMAKE_CXX_FLAGS=-Wsuggest-override

declaration of ‘RooFit::GetParametersPolicy policy’ shadows a parameter [-Werror=shadow]
policy.stripDisconnected = true;
_topNode->getParameters(nset, paramsStripped, policy);
RooArgSet toRemove;
for (RooAbsArg *param : outputSet) {
for (RooAbsArg *param : params) {
if (!paramsStripped.find(param->GetName())) {
toRemove.add(*param);
}
}
outputSet.remove(toRemove, /*silent*/ false, /*matchByNameOnly*/ true);
params.remove(toRemove, /*silent*/ false, /*matchByNameOnly*/ true);
}

return false;
}

/// @brief A wrapper class to store a C++ function of type 'double (*)(double*, double*)'.
Expand Down Expand Up @@ -779,7 +779,7 @@
{
// Get the parameters.
RooArgSet paramSet;
this->getParameters(_data ? _data->get() : nullptr, paramSet, /*sripDisconnectedParams=*/false);
this->getParameters(_data ? _data->get() : nullptr, paramSet);

const bool isChi2 = _topNode->getAttribute("Chi2EvaluationActive");
const bool skipZeroWeights = !isChi2 && (!_pdf || !_pdf->getAttribute("BinnedLikelihoodActive"));
Expand Down
Loading
Loading