Skip to content

[RF][HS3] Fixed snapshot filter, unbinned data edges, zero yield bins #22073

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
guitargeek merged 3 commits into from
May 8, 2026
+281 −178
Merged

[RF][HS3] Fixed snapshot filter, unbinned data edges, zero yield bins #22073

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
b87fe69
[RF][HS3] Fixed snapshot filter, unbinned data edges, zero yield bins
Phmonski Apr 27, 2026
9e55027
[RF][HS3] Add unit tests covering fixes in previous commit b87fe6909
guitargeek May 7, 2026
5f6bedc
[RF][HS3] Refactor to reduce code duplication and dead code
guitargeek May 7, 2026
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
40 changes: 19 additions & 21 deletions roofit/hs3/src/JSONFactories_HistFactory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,10 @@ class HistFactoryImporter : public RooFit::JSONIO::Importer {

std::vector<double> errs(sumW.size());
for (size_t i = 0; i < sumW.size(); ++i) {
if (sumW[i] == 0.) {
errs[i] = 0.;
continue;
}
errs[i] = std::sqrt(sumW2[i]) / sumW[i];
// avoid negative sigma. This NP will be set constant anyway later
errs[i] = std::max(errs[i], 0.);
Expand Down Expand Up @@ -1136,6 +1140,16 @@ void configureStatError(Channel &channel)

bool exportChannel(RooJSONFactoryWSTool *tool, const Channel &channel, JSONNode &elem)
{
// Write the constraint reference (either by name or by type) for any
// modifier that supports an external Gaussian/Poisson/etc. constraint.
auto writeConstraint = [](JSONNode &mod, auto const &sys) {
if (sys.constraint) {
mod["constraint_name"] << sys.constraint->GetName();
} else if (sys.constraintType) {
mod["constraint_type"] << toString(sys.constraintType);
}
};

bool observablesWritten = false;
for (const auto &sample : channel.samples) {

Expand Down Expand Up @@ -1167,11 +1181,7 @@ bool exportChannel(RooJSONFactoryWSTool *tool, const Channel &channel, JSONNode
if (sys.interpolationCode != 4) {
mod["interpolation"] << sys.interpolationCode;
}
if (sys.constraint) {
mod["constraint_name"] << sys.constraint->GetName();
} else if (sys.constraintType) {
mod["constraint_type"] << toString(sys.constraintType);
}
writeConstraint(mod, sys);
auto &data = mod["data"].set_map();
data["lo"] << sys.low;
data["hi"] << sys.high;
Expand All @@ -1183,11 +1193,7 @@ bool exportChannel(RooJSONFactoryWSTool *tool, const Channel &channel, JSONNode
mod["name"] << sys.name;
mod["type"] << "histosys";
mod["parameter"] << sys.param->GetName();
if (sys.constraint) {
mod["constraint_name"] << sys.constraint->GetName();
} else if (sys.constraintType) {
mod["constraint_type"] << toString(sys.constraintType);
}
writeConstraint(mod, sys);
auto &data = mod["data"].set_map();
if (channel.nBins != sys.low.size() || channel.nBins != sys.high.size()) {
std::stringstream ss;
Expand All @@ -1205,20 +1211,12 @@ bool exportChannel(RooJSONFactoryWSTool *tool, const Channel &channel, JSONNode
mod["name"] << sys.name;
mod["type"] << "shapesys";
optionallyExportGammaParameters(mod, sys.name, sys.parameters);
if (sys.constraint) {
mod["constraint_name"] << sys.constraint->GetName();
} else if (sys.constraintType) {
mod["constraint_type"] << toString(sys.constraintType);
}
writeConstraint(mod, sys);
auto &vals = mod["data"].set_map()["vals"];
if (sys.constraint || sys.constraintType) {
auto &vals = mod["data"].set_map()["vals"];
vals.fill_seq(sys.constraints);
} else {
auto &vals = mod["data"].set_map()["vals"];
vals.set_seq();
for (std::size_t i = 0; i < sys.parameters.size(); ++i) {
vals.append_child() << 0;
}
vals.fill_seq(std::vector<double>(sys.parameters.size(), 0.0));
}
}

Expand Down
58 changes: 25 additions & 33 deletions roofit/hs3/src/JSONFactories_RooFitCore.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -825,25 +825,30 @@ class RooFormulaArgStreamer : public RooFit::JSONIO::Exporter {
expr.ReplaceAll("TMath::Erf", "erf");
}
};
// Write the "x" reference and the coefficient list for polynomial-like
// pdfs/funcs, including the implicit defaults below "lowestOrder" so that the
// output is self-documenting.
template <class Pdf>
void writePolynomialBody(const Pdf *pdf, JSONNode &elem)
{
elem["x"] << pdf->x().GetName();
auto &coefs = elem["coefficients"].set_seq();
for (int i = 0; i < pdf->lowestOrder(); ++i) {
coefs.append_child() << (i == 0 ? "1.0" : "0.0");
}
for (const auto &coef : pdf->coefList()) {
coefs.append_child() << coef->GetName();
}
}

template <class RooArg_t>
class RooPolynomialStreamer : public RooFit::JSONIO::Exporter {
public:
std::string const &key() const override;
bool exportObject(RooJSONFactoryWSTool *, const RooAbsArg *func, JSONNode &elem) const override
{
auto *pdf = static_cast<const RooArg_t *>(func);
elem["type"] << key();
elem["x"] << pdf->x().GetName();
auto &coefs = elem["coefficients"].set_seq();
// Write out the default coefficient that RooFit uses for the lower
// orders before the order of the first coefficient. Like this, the
// output is more self-documenting.
for (int i = 0; i < pdf->lowestOrder(); ++i) {
coefs.append_child() << (i == 0 ? "1.0" : "0.0");
}
for (const auto &coef : pdf->coefList()) {
coefs.append_child() << coef->GetName();
}
writePolynomialBody(static_cast<const RooArg_t *>(func), elem);
return true;
}
};
Expand All @@ -853,19 +858,8 @@ class RooLegacyExpPolyStreamer : public RooFit::JSONIO::Exporter {
std::string const &key() const override;
bool exportObject(RooJSONFactoryWSTool *, const RooAbsArg *func, JSONNode &elem) const override
{
auto *pdf = static_cast<const RooLegacyExpPoly *>(func);
elem["type"] << key();
elem["x"] << pdf->x().GetName();
auto &coefs = elem["coefficients"].set_seq();
// Write out the default coefficient that RooFit uses for the lower
// orders before the order of the first coefficient. Like this, the
// output is more self-documenting.
for (int i = 0; i < pdf->lowestOrder(); ++i) {
coefs.append_child() << (i == 0 ? "1.0" : "0.0");
}
for (const auto &coef : pdf->coefList()) {
coefs.append_child() << coef->GetName();
}
writePolynomialBody(static_cast<const RooLegacyExpPoly *>(func), elem);
return true;
}
};
Expand Down Expand Up @@ -1102,18 +1096,16 @@ class ParamHistFuncStreamer : public RooFit::JSONIO::Exporter {
RooJSONFactoryWSTool::testValidName(name, false);
JSONNode &obsNode = observablesNode.append_child().set_map();
obsNode["name"] << name;
if (var->getBinning().isUniform()) {
auto const &binning = var->getBinning();
if (binning.isUniform()) {
obsNode["min"] << var->getMin();
obsNode["max"] << var->getMax();
obsNode["nbins"] << var->getBins();
} else {
auto &edges = obsNode["edges"];
edges.set_seq();
double val = var->getBinning().binLow(0);
edges.append_child() << val;
for (int i = 0; i < var->getBinning().numBins(); ++i) {
val = var->getBinning().binHigh(i);
edges.append_child() << val;
auto &edges = obsNode["edges"].set_seq();
edges.append_child() << binning.binLow(0);
for (int i = 0; i < binning.numBins(); ++i) {
edges.append_child() << binning.binHigh(i);
}
}
}
Expand Down Expand Up @@ -1256,7 +1248,7 @@ STATIC_EXECUTE([]() {
registerExporter<RooFFTConvPdfStreamer>(RooFFTConvPdf::Class(), false);
registerExporter<RooExtendPdfStreamer>(RooExtendPdf::Class(), false);
registerExporter<ParamHistFuncStreamer>(ParamHistFunc::Class(), false);
registerExporter<RooSplineStreamer>(RooSpline::Class(), false);
registerExporter<RooSplineStreamer>(RooSpline::Class(), false);
});

} // namespace
158 changes: 34 additions & 124 deletions roofit/hs3/src/RooJSONFactoryWSTool.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,9 @@ void RooJSONFactoryWSTool::exportVariable(const RooAbsArg *v, JSONNode &node, bo
var["value"] << rrv->getVal();
if (rrv->isConstant() && storeConstant) {
var["const"] << rrv->isConstant();
} else {
var["min"] << rrv->getMin();
var["max"] << rrv->getMax();
}
if (rrv->getBins() != 100 && storeBins) {
var["nbins"] << rrv->getBins();
Expand Down Expand Up @@ -1507,10 +1510,6 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data)
return;

JSONNode &output = appendNamedChild((*_rootnodeOutput)["data"], data.GetName());
/*std::ofstream file("/home/scello/Data/ZvvH126_5.txt", std::ios::app);
if (!file.is_open()) {
std::cerr << "Error: Could not open file for writing.\n";
}*/

// This works around a problem in RooStats/HistFactory that was only fixed
// in ROOT 6.30: until then, the weight variable of the observed dataset,
Expand Down Expand Up @@ -1832,35 +1831,21 @@ void RooJSONFactoryWSTool::exportSingleModelConfig(JSONNode &rootnode, RooFit::M

auto &domainsNode = rootnode["domains"];

if (mc.GetNuisanceParameters() && mc.GetNuisanceParameters()->size() > 0) {
std::string npDomainName = analysisName + "_nuisance_parameters";
domains.append_child() << npDomainName;
RooFit::JSONIO::Detail::Domains::ProductDomain npDomain;
for (auto *np : static_range_cast<const RooRealVar *>(*mc.GetNuisanceParameters())) {
npDomain.readVariable(*np);
}
npDomain.writeJSON(appendNamedChild(domainsNode, npDomainName));
}

if (mc.GetGlobalObservables() && mc.GetGlobalObservables()->size() > 0) {
std::string globDomainName = analysisName + "_global_observables";
domains.append_child() << globDomainName;
RooFit::JSONIO::Detail::Domains::ProductDomain globDomain;
for (auto *glob : static_range_cast<const RooRealVar *>(*mc.GetGlobalObservables())) {
globDomain.readVariable(*glob);
auto writeProductDomain = [&](const char *suffix, RooArgSet const *args) {
if (!args || args->empty())
return;
const std::string domainName = analysisName + suffix;
domains.append_child() << domainName;
RooFit::JSONIO::Detail::Domains::ProductDomain domain;
for (auto *var : static_range_cast<const RooRealVar *>(*args)) {
domain.readVariable(*var);
}
globDomain.writeJSON(appendNamedChild(domainsNode, globDomainName));
}
domain.writeJSON(appendNamedChild(domainsNode, domainName));
};

if (mc.GetParametersOfInterest() && mc.GetParametersOfInterest()->size() > 0) {
std::string poiDomainName = analysisName + "_parameters_of_interest";
domains.append_child() << poiDomainName;
RooFit::JSONIO::Detail::Domains::ProductDomain poiDomain;
for (auto *poi : static_range_cast<const RooRealVar *>(*mc.GetParametersOfInterest())) {
poiDomain.readVariable(*poi);
}
poiDomain.writeJSON(appendNamedChild(domainsNode, poiDomainName));
}
writeProductDomain("_nuisance_parameters", mc.GetNuisanceParameters());
writeProductDomain("_global_observables", mc.GetGlobalObservables());
writeProductDomain("_parameters_of_interest", mc.GetParametersOfInterest());

auto &modelConfigAux = getRooFitInternal(rootnode, "ModelConfigs", analysisName);
modelConfigAux.set_map();
Expand Down Expand Up @@ -1947,18 +1932,16 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n)
// the ones that the pdfs encoded implicitly (like in the case of
// HistFactory).
for (RooAbsArg *arg : *snsh) {
if (exportedObjectNames.find(arg->GetName()) != exportedObjectNames.end()) {
bool do_export = false;
for (const auto &pdf : allpdfs) {
if (pdf->dependsOn(*arg)) {
do_export = true;
}
}
if (do_export) {
RooJSONFactoryWSTool::testValidName(arg->GetName(), true);
snapshotSorted.add(*arg);
bool do_export = false;
for (const auto &pdf : allpdfs) {
if (pdf->dependsOn(*arg)) {
do_export = true;
}
}
if (do_export) {
RooJSONFactoryWSTool::testValidName(arg->GetName(), true);
snapshotSorted.add(*arg);
}
}
snapshotSorted.sort();
std::string name(snsh->GetName());
Expand Down Expand Up @@ -2478,44 +2461,6 @@ RooWorkspace RooJSONFactoryWSTool::cleanWS(const RooWorkspace &ws, bool onlyMode
tmpWS.import(*obj);
}

/*
if (auto* mc = dynamic_cast<RooFit::ModelConfig*>(obj)) {
// Import the PDF
tmpWS.import(*mc->GetPdf());

// Import all observables
RooArgSet* obs = (RooArgSet*)mc->GetObservables()->snapshot();
tmpWS.import(*obs);

// Import global observables
RooArgSet* globObs = (RooArgSet*)mc->GetGlobalObservables()->snapshot();
tmpWS.import(*globObs);

// Import POIs
RooArgSet* pois = (RooArgSet*)mc->GetParametersOfInterest()->snapshot();
tmpWS.import(*pois);

// Import nuisance parameters
RooArgSet* nuis = (RooArgSet*)mc->GetNuisanceParameters()->snapshot();
tmpWS.import(*nuis);


RooFit::ModelConfig* mc_new = new RooFit::ModelConfig(mc->GetName(), mc->GetName());

mc_new->SetPdf(*tmpWS.pdf(mc->GetPdf()->GetName()));
mc_new->SetObservables(*tmpWS.set(obs->GetName()));
mc_new->SetGlobalObservables(*tmpWS.set(globObs->GetName()));
mc_new->SetParametersOfInterest(*tmpWS.set(pois->GetName()));
mc_new->SetNuisanceParameters(*tmpWS.set(nuis->GetName()));

// Import the ModelConfig into the new workspace
tmpWS.import(*mc_new);
}else {

tmpWS.import(*obj);
}
*/

for (auto *snsh : ws.getSnapshots()) {
auto *snshSet = dynamic_cast<RooArgSet *>(snsh);
if (snshSet) {
Expand All @@ -2533,32 +2478,17 @@ RooWorkspace RooJSONFactoryWSTool::sanitizeWS(const RooWorkspace &ws)

RooWorkspace tmpWS = cleanWS(ws, false);

for (auto *obj : tmpWS.allVars()) {
if (!isValidName(obj->GetName())) {
obj->SetName(sanitizeName(obj->GetName()).c_str());
}
}

// Functions
for (auto *obj : tmpWS.allFunctions()) {
if (!isValidName(obj->GetName())) {
obj->SetName(sanitizeName(obj->GetName()).c_str());
}
}

// PDFs
for (auto *obj : tmpWS.allPdfs()) {
if (!isValidName(obj->GetName())) {
obj->SetName(sanitizeName(obj->GetName()).c_str());
}
}

// Resolution Models
for (auto *obj : tmpWS.allResolutionModels()) {
if (!isValidName(obj->GetName())) {
obj->SetName(sanitizeName(obj->GetName()).c_str());
auto sanitizeIfNeeded = [](auto const &list) {
for (auto *obj : list) {
if (!isValidName(obj->GetName())) {
obj->SetName(sanitizeName(obj->GetName()).c_str());
}
}
}
};
sanitizeIfNeeded(tmpWS.allVars());
sanitizeIfNeeded(tmpWS.allFunctions());
sanitizeIfNeeded(tmpWS.allPdfs());
sanitizeIfNeeded(tmpWS.allResolutionModels());
// Datasets
for (auto *data : tmpWS.allData()) {
// Sanitize dataset name
Expand All @@ -2569,26 +2499,6 @@ RooWorkspace RooJSONFactoryWSTool::sanitizeWS(const RooWorkspace &ws)
obj->SetName(sanitizeName(obj->GetName()).c_str());
}
}
/* // Sanitize dataset observables
const RooArgSet* obsSet = data->get();
if (obsSet) {
RooArgSet* mutableObs = const_cast<RooArgSet*>(obsSet);
std::string oldSetName = mutableObs->GetName();
std::string newSetName = sanitizeName(oldSetName);
if (oldSetName != newSetName) {
mutableObs->setName(newSetName.c_str());
}
}

for (auto* arg : *obsSet) {
std::string oldObsName = arg->GetName();
std::string newObsName = sanitizeName(oldObsName);
if (oldObsName != newObsName) {
arg->SetName(newObsName.c_str());
data->changeObservableName(arg->GetName(), newObsName.c_str());
}
}
*/
for (auto *data : tmpWS.allEmbeddedData()) {
// Sanitize dataset name
data->SetName(sanitizeName(data->GetName()).c_str());
Expand Down
Loading
Loading