From c4434a9d58dee966b542c7d589282803e198518b Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 15:54:00 +0100 Subject: [PATCH 01/18] [RF][HS3] Write RooSimultaneous as combined_dist and read it back A RooSimultaneous used to be exported only as ROOT-internal information (misc.ROOT_internal.combined_distributions). HS3 PR #119 defines the "combined_dist": its components are selected by a key with one label per column, and the columns are categorical axes of the domains. Export: a RooSimultaneous becomes a "combined_dist". The columns are its index category or, for a RooSuperCategory index, the input categories, sorted by name so that a distribution and a data set agree on the order without looking at each other. The columns are declared as categorical axes of the default domain. The components are written in the order of the columns and their states, which does not depend on the order of the inputs of a RooSuperCategory, so the document is stable under round trips. States without a pdf are not written. Category indices are not part of HS3: after a round trip, the index is the position in the domain's "categories". Import: a "combined_dist" becomes a RooSimultaneous over the column category, or over a RooSuperCategory of the columns in the order of the document. The columns must be declared by categorical axes of a domain; keys are validated against them. The ROOT-internal form of files written by earlier ROOT versions is still read. Tests: CombinedDist (structure, round trip, legacy form), CombinedDistSeveralColumns, CombinedDistImportErrors. Ported onto the typed HS3 document: the categories are recorded in the export state and written as categorical axes of the "default_domain", sorted by name together with the continuous axes. Assisted-by: claude-code:claude-fable-5-1 Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 4 + roofit/hs3/src/JSONFactories_RooFitCore.cxx | 44 +++++ roofit/hs3/src/JSONIOUtils.cxx | 153 +++++++++++++++ roofit/hs3/src/JSONIOUtils.h | 28 +++ roofit/hs3/src/RooJSONFactoryWSTool.cxx | 111 +++++++++-- roofit/hs3/test/testRooFitHS3.cxx | 184 ++++++++++++++++++ 6 files changed, 505 insertions(+), 19 deletions(-) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index 91e6e8389dd60..e624c5271bd6c 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -46,6 +46,8 @@ namespace RooStats { class ModelConfig; } class RooRealVar; +class RooAbsCategory; +class RooSimultaneous; class RooJSONFactoryWSTool { public: @@ -243,6 +245,8 @@ class RooJSONFactoryWSTool { } } + void exportSimultaneous(RooSimultaneous const &simPdf, std::set &exportedObjectNames); + void recordCategory(RooAbsCategory const &cat); void exportData(RooAbsData const &data); RooJSONFactoryWSTool::CombinedData exportCombinedData(RooAbsData const &data); diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index 7626180f718c6..66702515d67cb 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -57,6 +57,9 @@ #include +#include +#include +#include #include "JSONIOUtils.h" #include @@ -419,6 +422,46 @@ bool importAddition(RooJSONFactoryWSTool *tool, const JSONNode &p) return true; } +// A "combined_dist" is a RooSimultaneous. Its index category is the single column, or a RooSuperCategory over the +// columns in the order of the document; the columns are categories declared by categorical axes of the domains. +bool importCombinedDist(RooJSONFactoryWSTool *tool, const JSONNode &p) +{ + std::string const name(RooJSONFactoryWSTool::name(p)); + std::string const context = "the combined_dist '" + name + "'"; + std::vector columns = readSelectionColumns(*tool->workspace(), p, "columns", context); + + auto const *componentsNode = p.find("components"); + if (!componentsNode || !componentsNode->is_seq() || componentsNode->num_children() == 0) { + RooJSONFactoryWSTool::error("\"components\" of " + context + " must be a non-empty list"); + } + std::map pdfMap; + for (JSONNode const &component : componentsNode->children()) { + if (!component.is_map() || !component.has_child("dist")) { + RooJSONFactoryWSTool::error("every component of " + context + " must have a \"key\" and a \"dist\""); + } + std::vector const key = readSelectionKey(component, columns, context); + std::string const label = selectionLabel(key); + if (pdfMap.find(label) != pdfMap.end()) { + RooJSONFactoryWSTool::error("the key " + selectionKeyToString(key) + " is used by two components of " + + context); + } + pdfMap[label] = tool->request(component["dist"].val(), name); + } + + if (columns.size() == 1) { + tool->wsImport(RooSimultaneous{name.c_str(), name.c_str(), pdfMap, *columns.front()}); + return true; + } + RooArgSet inputs; + for (RooCategory *column : columns) { + inputs.add(*column); + } + std::string const indexName = name + "_index"; + RooSuperCategory indexCat{indexName.c_str(), indexName.c_str(), inputs}; + tool->wsImport(RooSimultaneous{name.c_str(), name.c_str(), pdfMap, indexCat}); + return true; +} + bool importAddPdf(RooJSONFactoryWSTool *tool, const JSONNode &p) { std::string name(RooJSONFactoryWSTool::name(p)); @@ -1640,6 +1683,7 @@ STATIC_EXECUTE([]() { registerImporter("sum", false); registerImporter("mixture_dist", false); registerImporter("mixture_model", false); + registerImporter("combined_dist", false); registerImporter("mixture_resolution_model", false); registerImporter("binsampling_dist", false); registerImporter("binsampling", false); diff --git a/roofit/hs3/src/JSONIOUtils.cxx b/roofit/hs3/src/JSONIOUtils.cxx index 7867a8b206197..07ca32c23c6e3 100644 --- a/roofit/hs3/src/JSONIOUtils.cxx +++ b/roofit/hs3/src/JSONIOUtils.cxx @@ -3,10 +3,16 @@ #include #include #include +#include +#include +#include #include #include +#include #include +#include +#include #include #include @@ -152,3 +158,150 @@ void checkParamHistFuncOrderIsDefined(std::string const &hs3Version, RooAbsColle "workspace with a newer ROOT version."); } } + +namespace { + +void sortByName(std::vector &columns) +{ + std::sort(columns.begin(), columns.end(), + [](RooCategory const *l, RooCategory const *r) { return std::strcmp(l->GetName(), r->GetName()) < 0; }); +} + +} // namespace + +std::vector selectionColumns(RooAbsCategory const &indexCat) +{ + std::vector columns; + auto addColumn = [&](RooAbsArg const &arg) { + auto *cat = dynamic_cast(&arg); + if (!cat) { + RooJSONFactoryWSTool::error("the index category '" + std::string(arg.GetName()) + "' is a " + arg.ClassName() + + ", but only a RooCategory or a RooSuperCategory of RooCategories " + + "can be written as the columns of a combined distribution"); + } + columns.push_back(cat); + }; + if (auto *super = dynamic_cast(&indexCat)) { + for (RooAbsArg const *input : super->inputCatList()) { + addColumn(*input); + } + } else { + addColumn(indexCat); + } + sortByName(columns); + return columns; +} + +std::vector selectionColumns(RooAbsCollection const &vars) +{ + std::vector columns; + for (RooAbsArg const *arg : vars) { + if (auto *cat = dynamic_cast(arg)) { + columns.push_back(cat); + } + } + sortByName(columns); + return columns; +} + +std::vector> selectionKeys(std::vector const &columns) +{ + std::vector> keys{{}}; + for (RooCategory const *column : columns) { + std::vector> next; + for (auto const &key : keys) { + for (unsigned int i = 0; i < column->size(); ++i) { + next.push_back(key); + next.back().push_back(column->getOrdinal(i).first); + } + } + keys = std::move(next); + } + return keys; +} + +std::string selectionLabel(std::vector const &key) +{ + if (key.size() == 1) { + return key.front(); + } + std::string label = "{"; + for (std::size_t i = 0; i < key.size(); ++i) { + label += (i == 0 ? "" : ";") + key[i]; + } + return label + "}"; +} + +std::string selectionKeyToString(std::vector const &key) +{ + std::string out = "["; + for (std::size_t i = 0; i < key.size(); ++i) { + out += (i == 0 ? "\"" : ", \"") + key[i] + "\""; + } + return out + "]"; +} + +std::string selectionLabel(RooAbsCategory const &indexCat, std::vector const &columns, + std::vector const &key) +{ + auto *super = dynamic_cast(&indexCat); + if (!super) { + return key.front(); + } + std::vector inInputOrder; + for (RooAbsArg const *input : super->inputCatList()) { + auto found = std::find_if(columns.begin(), columns.end(), + [&](RooCategory const *c) { return std::strcmp(c->GetName(), input->GetName()) == 0; }); + inInputOrder.push_back(key[found - columns.begin()]); + } + return selectionLabel(inInputOrder); +} + +std::vector +readSelectionColumns(RooWorkspace const &ws, JSONNode const &node, const char *field, std::string const &context) +{ + auto const *list = node.find(field); + if (!list || !list->is_seq() || list->num_children() == 0) { + RooJSONFactoryWSTool::error("\"" + std::string(field) + "\" of " + context + + " must be a non-empty list of names of categorical axes"); + } + std::vector columns; + for (JSONNode const &child : list->children()) { + if (child.is_container() || child.val().empty()) { + RooJSONFactoryWSTool::error("\"" + std::string(field) + "\" of " + context + + " must be a non-empty list of names of categorical axes"); + } + RooCategory *cat = ws.cat(child.val()); + if (!cat) { + RooJSONFactoryWSTool::error("the column '" + child.val() + "' of " + context + + " is not declared by a categorical axis of a domain"); + } + if (std::find(columns.begin(), columns.end(), cat) != columns.end()) { + RooJSONFactoryWSTool::error("the column '" + child.val() + "' of " + context + " is listed twice"); + } + columns.push_back(cat); + } + return columns; +} + +std::vector +readSelectionKey(JSONNode const &component, std::vector const &columns, std::string const &context) +{ + auto const *keyNode = component.find("key"); + if (!keyNode || !keyNode->is_seq() || keyNode->num_children() != columns.size()) { + RooJSONFactoryWSTool::error("every component of " + context + " must have a \"key\" with " + + std::to_string(columns.size()) + " labels, one per column"); + } + std::vector key; + std::size_t i = 0; + for (JSONNode const &label : keyNode->children()) { + if (label.is_container() || !columns[i]->hasLabel(label.val())) { + RooJSONFactoryWSTool::error("the label '" + (label.is_container() ? std::string("") : label.val()) + + "' in position " + std::to_string(i) + " of a key of " + context + + " is not one of the categories of '" + columns[i]->GetName() + "'"); + } + key.push_back(label.val()); + ++i; + } + return key; +} diff --git a/roofit/hs3/src/JSONIOUtils.h b/roofit/hs3/src/JSONIOUtils.h index 573dcd92963b8..cf231c5c1c2c5 100644 --- a/roofit/hs3/src/JSONIOUtils.h +++ b/roofit/hs3/src/JSONIOUtils.h @@ -6,9 +6,12 @@ #include class RooAbsBinning; +class RooAbsCategory; class RooAbsCollection; class RooAbsReal; class RooArgList; +class RooCategory; +class RooWorkspace; class ParamHistFunc; bool startsWith(std::string_view str, std::string_view prefix); @@ -31,4 +34,29 @@ RooArgList toParamHistFuncOrder(RooArgList const ¶msInBinOrder, RooAbsCollec void checkParamHistFuncOrderIsDefined(std::string const &hs3Version, RooAbsCollection const &vars, std::string const &context); +// Selection keys (HS3 "combined" data sets and "combined_dist"): a key is one label per column, and the columns are +// categories declared by categorical axes of the domains. In RooFit, the columns are the index category of a +// RooSimultaneous (a RooCategory), or the input categories of a RooSuperCategory index, or the category observables +// of a combined data set. They are sorted by name, so that a distribution and a data set agree on the order of the +// columns without looking at each other. +std::vector selectionColumns(RooAbsCategory const &indexCat); +std::vector selectionColumns(RooAbsCollection const &vars); +// All keys of the columns, in the order of the columns and of their states, with the first column varying slowest. +std::vector> selectionKeys(std::vector const &columns); +// The state label of a RooCategory (one column) or of a RooSuperCategory over the columns in that order. +std::string selectionLabel(std::vector const &key); +// The state label of `indexCat` for a key with the labels in the order of `columns`: the label itself for a +// RooCategory, or "{a;b}" with the labels in the order of the inputs for a RooSuperCategory. +std::string selectionLabel(RooAbsCategory const &indexCat, std::vector const &columns, + std::vector const &key); +// A key as written in the document, for messages: ["a", "b"]. +std::string selectionKeyToString(std::vector const &key); +// Read the columns of a combined distribution or data set from the list `field` of `node`: each entry must be the +// name of a category in the workspace, i.e. declared by a categorical axis of a domain. +std::vector readSelectionColumns(RooWorkspace const &ws, RooFit::Detail::JSONNode const &node, + const char *field, std::string const &context); +// Read and validate the "key" of a component: one label per column, each one of the categories of its column. +std::vector readSelectionKey(RooFit::Detail::JSONNode const &component, + std::vector const &columns, std::string const &context); + #endif diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 46ab5b48bb9c8..526e315d8ad84 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -157,6 +157,8 @@ struct RooFit::HS3::ToolExportState { JSONNode *root = nullptr; /// The ranges and binnings of the exported variables, by domain and variable name. std::map> domains; + /// The categories that are declared by categorical axes of the "default_domain", by name. + std::map categories; /// Set by the exporter of an object that is interpreted by the document. std::optional typedObject; std::vector channelConstraints; @@ -213,12 +215,21 @@ void recordDomains(std::map> } } +// Append a domain with the axes in the order of their names, continuous and categorical ones mixed. void appendDomain(std::vector &domains, std::string const &name, - std::map axes) + std::map axes, + std::map categories = {}) { + std::map> sorted; + for (auto &item : axes) { + sorted.emplace(item.first, std::move(item.second)); + } + for (auto &item : categories) { + sorted.emplace(item.first, std::move(item.second)); + } HS3::Domain &domain = domains.emplace_back(); domain.name = name; - for (auto &item : axes) { + for (auto &item : sorted) { domain.axes.emplace_back(std::move(item.second)); } } @@ -1459,21 +1470,11 @@ void RooJSONFactoryWSTool::exportObject(RooAbsArg const &func, std::set(&func)) { - // RooSimultaneous is not used in the HS3 standard, we only export the - // dependents and some ROOT internal information. - exportObjects(func.servers(), exportedObjectNames); - - HS3::CombinedDistribution &info = _export->doc.rooFit.combinedDistributions[simPdf->GetName()]; - info.indexCat = simPdf->indexCat().GetName(); - for (auto const &item : simPdf->indexCat()) { - info.labels.push_back(makeValidNameOrError(item.first)); - info.indices.push_back(item.second); - info.distributions.push_back({simPdf->getPdf(item.first.c_str())->GetName()}); - } - + exportSimultaneous(*simPdf, exportedObjectNames); return; } else if (dynamic_cast(&func)) { - // categories are created by the respective RooSimultaneous, so we're skipping the export here + // Categories are not objects of the document: the index category of a RooSimultaneous is declared by the + // categorical axes of the domain that its "columns" name. return; } else if (dynamic_cast(&func) || dynamic_cast(&func)) { exportVariable(&func, _export->doc.parameterPoints.front(), true); @@ -1598,6 +1599,62 @@ void RooJSONFactoryWSTool::exportObject(RooAbsArg const &func, std::set &exportedObjectNames) +{ + RooAbsCategory const &indexCat = simPdf.indexCat(); + std::vector columns = selectionColumns(indexCat); + + JSONNode &elem = appendNamedChild((*_export->root)["distributions"], simPdf.GetName()); + elem["type"] << "combined_dist"; + auto &columnsNode = elem["columns"].set_seq(); + for (RooCategory const *column : columns) { + columnsNode.append_child() << column->GetName(); + recordCategory(*column); + } + auto &components = elem["components"].set_seq(); + // In the order of the columns and of their states, which is the order in which the categories are written to + // the domain. This does not depend on the order of the inputs of a RooSuperCategory, so the document is the same + // after a round trip. + for (std::vector const &key : selectionKeys(columns)) { + RooAbsPdf const *pdf = simPdf.getPdf(selectionLabel(indexCat, columns, key)); + if (!pdf) { + continue; + } + JSONNode &component = components.append_child().set_map(); + component["key"].fill_seq(key); + component["dist"] << pdf->GetName(); + } + recordGenericObject(_export->doc, true, simPdf.GetName(), elem); + + exportObjects(simPdf.servers(), exportedObjectNames); +} + +// Declare a category as a categorical axis of the "default_domain". The states are written in their insertion order: +// this is the order of the bins of a RooDataHist over the category, and the position becomes the index when the +// document is read back. +void RooJSONFactoryWSTool::recordCategory(RooAbsCategory const &cat) +{ + HS3::CategoricalAxis axis; + axis.name = cat.GetName(); + for (unsigned int i = 0; i < cat.size(); ++i) { + axis.categories.push_back(cat.getOrdinal(i).first); + } + auto found = _export->categories.find(axis.name); + if (found != _export->categories.end() && found->second.categories != axis.categories) { + RooJSONFactoryWSTool::error("the category '" + axis.name + + "' is written to the domain twice, with different states"); + } + _export->categories[axis.name] = std::move(axis); +} + /** * @brief Import a function from the JSONNode into the workspace. * @@ -1864,11 +1921,17 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA // Find a RooSimultaneous model that would fit to this dataset RooSimultaneous const *simPdf = nullptr; - for (auto const &item : _export->doc.rooFit.combinedDistributions) { - if (item.second.indexCat == cat->GetName()) { - simPdf = static_cast(_workspace.pdf(item.first)); + std::vector candidates; + for (RooAbsArg *arg : _workspace.allPdfs()) { + auto *candidate = dynamic_cast(arg); + if (candidate && std::string(candidate->indexCat().GetName()) == cat->GetName()) { + candidates.push_back(candidate); } } + if (!candidates.empty()) { + sortByName(candidates); + simPdf = candidates.front(); + } // If there is an associated simultaneous pdf for the index category, we // use the RooAbsData::split() overload that takes the RooSimultaneous. @@ -2308,11 +2371,21 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) this->exportVariables(snapshotSorted, doc.parameterPoints.back(), true); } } + // The categories are categorical axes of the "default_domain", which must not list them as continuous. + auto &defaultAxes = _export->domains[HS3::defaultDomainName]; + for (auto const &item : _export->categories) { + if (defaultAxes.count(item.first)) { + RooJSONFactoryWSTool::error("the variable '" + item.first + + "' is categorical, but the domain already lists it as continuous"); + } + } // The domains of the analyses take precedence. for (auto &item : _export->domains) { if (std::none_of(doc.domains.begin(), doc.domains.end(), [&](HS3::Domain const &domain) { return domain.name == item.first; })) { - appendDomain(doc.domains, item.first, std::move(item.second)); + appendDomain(doc.domains, item.first, std::move(item.second), + item.first == HS3::defaultDomainName ? std::move(_export->categories) + : std::map{}); } } diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index fd40fc38d7097..c3312ed4e6a3c 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include @@ -5497,3 +5498,186 @@ TEST(RooFitHS3, HistFactoryMultiDimensionalBinOrder) })", "Documents before HS3 0.3 don't define the order"); } + +// A RooSimultaneous is a "combined_dist" (HS3 #119): the components are selected by a key with one label per column, +// and the columns are declared as categorical axes of the default domain. The ROOT-internal form of older files is +// still read. +TEST(RooFitHS3, CombinedDist) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::model(x[-8, 8], mean[0, -8, 8], sigma[0.3, 0.1, 10])"); + ws.factory("Gaussian::model_ctl(x, mean, 1.0)"); + // The states are defined in this order, with non-contiguous indices. + ws.factory("SIMUL::simPdf(sample[physics=3, control=7], physics=model, control=model_ctl)"); + + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const &root = tree->rootnode(); + + // Structure of the written document. + auto const *simPdfNode = RooJSONFactoryWSTool::findNamedChild(root["distributions"], "simPdf"); + ASSERT_NE(simPdfNode, nullptr) << json; + EXPECT_EQ((*simPdfNode)["type"].val(), "combined_dist"); + ASSERT_EQ((*simPdfNode)["columns"].num_children(), 1u); + EXPECT_EQ((*simPdfNode)["columns"].child(0).val(), "sample"); + auto const &components = (*simPdfNode)["components"]; + ASSERT_EQ(components.num_children(), 2u); + EXPECT_EQ(components.child(0)["key"].num_children(), 1u); + EXPECT_EQ(components.child(0)["key"].child(0).val(), "physics"); + EXPECT_EQ(components.child(0)["dist"].val(), "model"); + EXPECT_EQ(components.child(1)["key"].child(0).val(), "control"); + EXPECT_EQ(components.child(1)["dist"].val(), "model_ctl"); + EXPECT_FALSE(root.find("misc", "ROOT_internal", "combined_distributions")) << json; + // The column is a categorical axis of the default domain, with the states in their order. + auto const *domain = RooJSONFactoryWSTool::findNamedChild(root["domains"], "default_domain"); + ASSERT_NE(domain, nullptr); + auto const *axis = RooJSONFactoryWSTool::findNamedChild((*domain)["axes"], "sample"); + ASSERT_NE(axis, nullptr) << json; + ASSERT_EQ((*axis)["categories"].num_children(), 2u); + EXPECT_EQ((*axis)["categories"].child(0).val(), "physics"); + EXPECT_EQ((*axis)["categories"].child(1).val(), "control"); + + // Round trip: the same RooSimultaneous, with the index category from the domain (indices = positions). + EXPECT_EQ(validate(ws, "simPdf"), 0); + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(json)); + auto *simPdf = dynamic_cast(ws2.pdf("simPdf")); + ASSERT_NE(simPdf, nullptr); + EXPECT_STREQ(simPdf->indexCat().GetName(), "sample"); + EXPECT_EQ(simPdf->indexCat().lookupIndex("physics"), 0); + EXPECT_EQ(simPdf->indexCat().lookupIndex("control"), 1); + EXPECT_STREQ(simPdf->getPdf("physics")->GetName(), "model"); + EXPECT_STREQ(simPdf->getPdf("control")->GetName(), "model_ctl"); + + // Files written by earlier ROOT versions describe the RooSimultaneous in misc.ROOT_internal. + RooWorkspace wsLegacy; + ASSERT_TRUE(RooJSONFactoryWSTool{wsLegacy}.importJSONfromString(R"({ + "metadata": {"hs3_version": "0.2"}, + "domains": [{"name": "default_domain", "type": "product_domain", + "axes": [{"name": "x", "min": -8.0, "max": 8.0}, {"name": "mean", "min": -8.0, "max": 8.0}]}], + "distributions": [ + {"name": "model", "type": "gaussian_dist", "x": "x", "mean": "mean", "sigma": 0.3}, + {"name": "model_ctl", "type": "gaussian_dist", "x": "x", "mean": "mean", "sigma": 1.0} + ], + "misc": {"ROOT_internal": {"combined_distributions": {"simPdf": { + "index_cat": "sample", "labels": ["physics", "control"], "indices": [3, 7], + "distributions": ["model", "model_ctl"]}}}} + })")); + auto *legacySimPdf = dynamic_cast(wsLegacy.pdf("simPdf")); + ASSERT_NE(legacySimPdf, nullptr); + EXPECT_EQ(legacySimPdf->indexCat().lookupIndex("control"), 7); + EXPECT_STREQ(legacySimPdf->getPdf("control")->GetName(), "model_ctl"); + // Its export is in the standard form. + const std::string fromLegacy = RooJSONFactoryWSTool{wsLegacy}.exportJSONtoString(); + EXPECT_NE(fromLegacy.find(R"("type":"combined_dist")"), std::string::npos) << fromLegacy; + EXPECT_EQ(fromLegacy.find("combined_distributions"), std::string::npos) << fromLegacy; +} + +// A RooSimultaneous over a RooSuperCategory has several columns, sorted by name, and keys with one label per column. +TEST(RooFitHS3, CombinedDistSeveralColumns) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooRealVar x{"x", "x", 0.0, 10.0}; + x.setBins(5); + RooGaussian sr{"sr", "sr", x, 3.0, 1.0}; + RooGaussian cr{"cr", "cr", x, 6.0, 1.0}; + RooGaussian srGlobal{"sr_global", "sr_global", x, 5.0, 2.0}; + // The super category is built over (region, kind); the columns are written sorted by name: (kind, region). + RooCategory region{"region", "region", {{"SR", 0}, {"CR", 1}}}; + RooCategory kind{"kind", "kind", {{"physics", 0}, {"global", 1}}}; + RooSuperCategory index{"index", "index", RooArgSet{region, kind}}; + // Three of the four keys have a component. + RooSimultaneous simPdf{ + "simPdf", "simPdf", {{"{SR;physics}", &sr}, {"{CR;physics}", &cr}, {"{SR;global}", &srGlobal}}, index}; + + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const *simPdfNode = RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["distributions"], "simPdf"); + ASSERT_NE(simPdfNode, nullptr) << json; + ASSERT_EQ((*simPdfNode)["columns"].num_children(), 2u); + EXPECT_EQ((*simPdfNode)["columns"].child(0).val(), "kind"); + EXPECT_EQ((*simPdfNode)["columns"].child(1).val(), "region"); + std::map> keys; + for (auto const &component : (*simPdfNode)["components"].children()) { + ASSERT_EQ(component["key"].num_children(), 2u); + keys[component["dist"].val()] = {component["key"].child(0).val(), component["key"].child(1).val()}; + } + const std::map> expectedKeys{ + {"sr", {"physics", "SR"}}, {"cr", {"physics", "CR"}}, {"sr_global", {"global", "SR"}}}; + EXPECT_EQ(keys, expectedKeys) << json; + + // Round trip: the super category is rebuilt over the columns, and every component evaluates to the same values. + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(json)); + auto *simPdf2 = dynamic_cast(ws2.pdf("simPdf")); + ASSERT_NE(simPdf2, nullptr); + ASSERT_NE(dynamic_cast(&simPdf2->indexCat()), nullptr); + EXPECT_EQ(RooJSONFactoryWSTool{ws2}.exportJSONtoString(), json); + RooArgSet nset{x}; + RooArgSet nset2{*ws2.var("x")}; + for (auto const &item : expectedKeys) { + kind.setLabel(item.second[0]); + region.setLabel(item.second[1]); + ws2.cat("kind")->setLabel(item.second[0]); + ws2.cat("region")->setLabel(item.second[1]); + EXPECT_STREQ(simPdf2->getPdf(simPdf2->indexCat().getCurrentLabel())->GetName(), item.first.c_str()); + for (int i = 0; i < x.numBins(); ++i) { + x.setBin(i); + ws2.var("x")->setBin(i); + EXPECT_DOUBLE_EQ(simPdf2->getVal(nset2), simPdf.getVal(nset)) << item.first << " bin " << i; + } + } +} + +TEST(RooFitHS3, CombinedDistImportErrors) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + auto makeJSON = [](std::string const &columns, std::string const &components, + std::string const &axes = R"({"name": "sample", "categories": ["a", "b"]})") { + return R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", + "axes": [{"name": "x", "min": 0.0, "max": 10.0}, )" + + axes + R"(]}], + "distributions": [ + {"name": "ga", "type": "gaussian_dist", "x": "x", "mean": 3.0, "sigma": 1.0}, + {"name": "gb", "type": "gaussian_dist", "x": "x", "mean": 6.0, "sigma": 1.0}, + {"name": "simPdf", "type": "combined_dist", "columns": )" + + columns + R"(, "components": )" + components + R"(} + ] + })"; + }; + const std::string good = R"([{"key": ["a"], "dist": "ga"}, {"key": ["b"], "dist": "gb"}])"; + { + RooWorkspace ws; + ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString(makeJSON(R"(["sample"])", good))); + EXPECT_NE(dynamic_cast(ws.pdf("simPdf")), nullptr); + } + expectImportThrowsWithError( + makeJSON(R"(["channel"])", good), + "the column 'channel' of the combined_dist 'simPdf' is not declared by a categorical axis"); + expectImportThrowsWithError(makeJSON(R"([])", good), "must be a non-empty list of names of categorical axes"); + expectImportThrowsWithError(makeJSON(R"(["sample", "sample"])", R"([{"key": ["a", "a"], "dist": "ga"}])"), + "the column 'sample' of the combined_dist 'simPdf' is listed twice"); + expectImportThrowsWithError(makeJSON(R"(["sample"])", R"([{"key": ["a", "b"], "dist": "ga"}])"), + "must have a \"key\" with 1 labels"); + expectImportThrowsWithError(makeJSON(R"(["sample"])", R"([{"key": ["c"], "dist": "ga"}])"), + "the label 'c' in position 0 of a key of the combined_dist 'simPdf' is not one of the " + "categories of 'sample'"); + expectImportThrowsWithError( + makeJSON(R"(["sample"])", R"([{"key": ["a"], "dist": "ga"}, {"key": ["a"], "dist": "gb"}])"), + "the key [\"a\"] is used by two components"); + expectImportThrowsWithError(makeJSON(R"(["sample"])", R"([])"), + "\"components\" of the combined_dist 'simPdf' must be a non-empty list"); + expectImportThrowsWithError(makeJSON(R"(["sample"])", R"([{"key": ["a"]}])"), "must have a \"key\" and a \"dist\""); + // The column must be categorical. + expectImportThrowsWithError( + makeJSON(R"(["sample"])", good, R"({"name": "sample", "min": 0.0, "max": 1.0})"), + "the column 'sample' of the combined_dist 'simPdf' is not declared by a categorical axis"); +} From 87de29d8a54be623e0071b6c591e3f0a07d7a6d6 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 16:09:14 +0100 Subject: [PATCH 02/18] [RF][HS3] Write combined data sets in the HS3 form and read them back A RooDataSet with a category observable used to be written as its components plus ROOT-internal information on how to combine them (misc.ROOT_internal.combined_datasets), and the likelihood of a ModelConfig listed the components of the RooSimultaneous paired with the components of the data. HS3 PR #119 defines the "combined" data set: sub-data sets selected by a key, with the columns declared as categorical axes of the domains. Export: a data set whose observables include RooCategories becomes a "combined" entry with the categories as columns, sorted by name, and one sub-data set per key, named after the data set and the labels. The categories are declared in the default domain. A key is written also without entries, since RooFit treats a channel without entries as present and empty. If the workspace has a RooSimultaneous with the same columns, each sub-data set carries only the observables of its component, as before, and keys without entries for which it has no component are left out; otherwise all observables are written. The likelihood of a ModelConfig now pairs the RooSimultaneous with the combined data set, and an analysis is written for every combined data set with the same columns. A RooSimultaneous without matching data gets an analysis named after the ModelConfig, like any other pdf, instead of one named after the pdf. Import: a "combined" data set becomes a RooDataSet with one category column per entry of "columns", filled from the components, which are then not imported on their own. One code path for one and for several columns. The ModelConfig observables then include the index category. The ROOT-internal form of files written by earlier ROOT versions is still read; its likelihoods list the components, so it is handled after the analyses as before. An empty weighted data set over a variable without bins was detected as binned data with zero bins, which made the document of a present but empty sub-data set unstable under round trips. The detection now requires a binning. Tests: CombinedData (structure, round trip, data without pdf, legacy form), CombinedDataSeveralColumns, CombinedDataImportErrors. Ported onto the typed HS3 document: a combined data set is a CombinedData in HS3::Data. The parser checks its structure, the validation checks the columns against the categorical axes of the domains, the labels of the keys, and that each sub-data set is a data set of the document that belongs to one combined data set. The likelihood of a ModelConfig names the RooSimultaneous, and the constraint terms of its HistFactory channels are still listed in "aux_distributions". Assisted-by: claude-code:claude-fable-5-1 Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 6 +- roofit/hs3/src/HS3Document.h | 14 +- roofit/hs3/src/HS3Parser.cxx | 58 +++ roofit/hs3/src/HS3Validate.cxx | 70 ++++ roofit/hs3/src/HS3Writer.cxx | 10 + roofit/hs3/src/RooJSONFactoryWSTool.cxx | 352 +++++++++++------- roofit/hs3/test/testRooFitHS3.cxx | 252 +++++++++++++ 7 files changed, 625 insertions(+), 137 deletions(-) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index e624c5271bd6c..e1ae803268089 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -70,7 +70,7 @@ class RooJSONFactoryWSTool { struct CombinedData { std::string name; - std::map components; + std::vector columns; }; RooJSONFactoryWSTool(RooWorkspace &ws); @@ -207,8 +207,6 @@ class RooJSONFactoryWSTool { static void exportArray(std::size_t n, double const *contents, RooFit::Detail::JSONNode &output); - void exportCategory(RooAbsCategory const &cat, RooFit::Detail::JSONNode &node); - void queueExport(RooAbsArg const &arg) { _serversToExport.push_back(&arg); } void queueExportTemporary(RooAbsArg *arg) { @@ -267,7 +265,7 @@ class RooJSONFactoryWSTool { const std::vector &single); void exportSingleModelConfig(RooStats::ModelConfig const &mc, std::string const &analysisName, - std::map const *dataComponents); + std::string const *dataName); friend void RooFit::HS3::setExportedObject(RooJSONFactoryWSTool &tool, RooFit::HS3::HistFactoryDist dist, std::vector channelConstraints); diff --git a/roofit/hs3/src/HS3Document.h b/roofit/hs3/src/HS3Document.h index 317e558a0d1e3..5709a3f357b51 100644 --- a/roofit/hs3/src/HS3Document.h +++ b/roofit/hs3/src/HS3Document.h @@ -188,9 +188,21 @@ struct BinnedData { std::optional uncertainty; }; +/// The sub-data set of a combined data set for one key, which has one label per column. +struct CombinedDataComponent { + std::vector key; + DataRef data; +}; + +/// Data sets that are selected by a key (HS3 #119). The columns are categorical axes of the domains. +struct CombinedData { + std::vector columns; + std::vector components; +}; + struct Data { std::string name; - std::variant body; + std::variant body; }; struct LikelihoodTerm { diff --git a/roofit/hs3/src/HS3Parser.cxx b/roofit/hs3/src/HS3Parser.cxx index 739cc84951863..78b622b2da90a 100644 --- a/roofit/hs3/src/HS3Parser.cxx +++ b/roofit/hs3/src/HS3Parser.cxx @@ -574,6 +574,60 @@ void describe(Reader &ar, BinnedData &data) } } +// A combined data set. Only the structure is checked here: the columns and keys are checked against the domains by the +// validation, and the sub-data sets when the data set is built. +void readCombinedData(Reader &ar, std::string const &name, CombinedData &data) +{ + std::string const context = "the combined data set '" + name + "'"; + Cursor const &c = ar.cursor(); + if (ar.has("axes")) { + c.error(context + " must not have \"axes\": its columns are declared by the domains"); + return; + } + auto nonEmptyList = [&](const char *key) -> JSONNode const * { + if (!ar.has(key) || !c.node()[key].is_seq() || c.node()[key].num_children() == 0) { + return nullptr; + } + return &c.node()[key]; + }; + JSONNode const *columns = nonEmptyList("columns"); + for (std::size_t i = 0; columns && i < columns->num_children(); ++i) { + JSONNode const &column = columns->child(i); + if (column.is_container() || column.val().empty()) { + columns = nullptr; + break; + } + data.columns.push_back(column.val()); + } + if (!columns) { + c.error("\"columns\" of " + context + " must be a non-empty list of names of categorical axes"); + return; + } + JSONNode const *components = nonEmptyList("components"); + if (!components) { + c.error("\"components\" of " + context + " must be a non-empty list"); + return; + } + for (JSONNode const &component : components->children()) { + if (!component.is_map() || !component.has_child("key") || !component.has_child("dataset")) { + c.error("every component of " + context + " must have a \"key\" and a \"dataset\""); + return; + } + JSONNode const &key = component["key"]; + CombinedDataComponent &out = data.components.emplace_back(); + if (key.is_seq() && key.num_children() == data.columns.size()) { + for (JSONNode const &label : key.children()) { + out.key.push_back(label.is_container() ? std::string{} : label.val()); + } + } else { + c.error("every component of " + context + " must have a \"key\" with " + std::to_string(data.columns.size()) + + " labels, one per column"); + return; + } + out.data.name = component["dataset"].val(); + } +} + void readValue(Cursor const &c, Data &out) { if (!c.node().is_map()) { @@ -596,6 +650,10 @@ void readValue(Cursor const &c, Data &out) PointData body; describe(ar, body); out.body = std::move(body); + } else if (type == "combined") { + CombinedData body; + readCombinedData(ar, out.name, body); + out.body = std::move(body); } else if (ar.has("type")) { c.error("unknown type of dataset '" + type + "'"); } diff --git a/roofit/hs3/src/HS3Validate.cxx b/roofit/hs3/src/HS3Validate.cxx index 65fa4d02952b0..62abbe43515d9 100644 --- a/roofit/hs3/src/HS3Validate.cxx +++ b/roofit/hs3/src/HS3Validate.cxx @@ -45,6 +45,7 @@ class Validator { checkLikelihoods(); checkAnalyses(); checkData(); + checkCombinedData(); for (std::size_t i = 0; i < _doc.distributions.size(); ++i) { auto const &dist = _doc.distributions[i]; if (auto const *histFactory = std::get_if(&dist.body)) { @@ -130,6 +131,7 @@ class Validator { void checkLikelihoods(); void checkAnalyses(); void checkData(); + void checkCombinedData(); HS3Document const &_doc; Diagnostics &_diagnostics; @@ -329,6 +331,74 @@ void Validator::checkCategoricalAxes() } } +// The columns of combined data sets are categorical axes of the domains, or categories that exist already, and the keys +// consist of their categories. Every sub-data set is a data set of the document that belongs to one combined data set. +void Validator::checkCombinedData() +{ + std::map const *> categorical; + for (Domain const &domain : _doc.domains) { + for (auto const &axis : domain.axes) { + if (auto const *cat = std::get_if(&axis)) { + categorical.emplace(cat->name, &cat->categories); + } + } + } + std::set combinedNames; + for (auto const &data : _doc.data) { + if (std::holds_alternative(data.body)) { + combinedNames.insert(data.name); + } + } + std::set usedComponents; + for (std::size_t i = 0; i < _doc.data.size(); ++i) { + auto const *combined = std::get_if(&_doc.data[i].body); + if (!combined) { + continue; + } + std::string const path = at("data", i, _doc.data[i].name); + std::string const context = "the combined data set '" + _doc.data[i].name + "'"; + std::vector const *> columnCategories; + std::set columns; + for (std::string const &column : combined->columns) { + auto found = categorical.find(column); + if (found == categorical.end() && !isExternal(column)) { + error(path, + "the column '" + column + "' of " + context + " is not declared by a categorical axis of a domain"); + return; + } + if (!columns.insert(column).second) { + error(path, "the column '" + column + "' of " + context + " is listed twice"); + return; + } + columnCategories.push_back(found != categorical.end() ? found->second : nullptr); + } + std::set> keys; + for (CombinedDataComponent const &component : combined->components) { + for (std::size_t k = 0; k < component.key.size(); ++k) { + auto const *categories = columnCategories[k]; + if (categories && + std::find(categories->begin(), categories->end(), component.key[k]) == categories->end()) { + error(path, "the label '" + component.key[k] + "' in position " + std::to_string(k) + " of a key of " + + context + " is not one of the categories of '" + combined->columns[k] + "'"); + } + } + if (!keys.insert(component.key).second) { + std::string key = "["; + for (std::size_t k = 0; k < component.key.size(); ++k) { + key += (k == 0 ? "\"" : ", \"") + component.key[k] + "\""; + } + error(path, "the key " + key + "] is used by two components of " + context); + } + std::string const &name = component.data.name; + if (!isDefinedIn(name, "data") || combinedNames.count(name) || !usedComponents.insert(name).second) { + error(path, "the component '" + name + "' of " + context + + " is not a data set of the document, or it is already a component of another combined " + "data set"); + } + } + } +} + void Validator::checkLikelihoods() { for (std::size_t i = 0; i < _doc.likelihoods.size(); ++i) { diff --git a/roofit/hs3/src/HS3Writer.cxx b/roofit/hs3/src/HS3Writer.cxx index f1a92dc8afeee..58d791ce43b62 100644 --- a/roofit/hs3/src/HS3Writer.cxx +++ b/roofit/hs3/src/HS3Writer.cxx @@ -272,6 +272,16 @@ void writeData(JSONNode &dataArray, Data const &data) node["weights"].fill_seq(*d.weights); } }, + [&](CombinedData const &d) { + node["type"] << "combined"; + writeStrings(node["columns"], d.columns); + JSONNode &components = node["components"].set_seq(); + for (CombinedDataComponent const &component : d.components) { + JSONNode &out = components.append_child().set_map(); + writeStrings(out["key"], component.key); + out["dataset"] << component.data.name; + } + }, [&](PointData const &d) { node["type"] << "point"; node["value"] << d.value; diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 526e315d8ad84..992fea78b772a 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -390,12 +391,6 @@ void RooFit::HS3::report(Diagnostics const &diagnostics) namespace { -// True if the number of components in `data` matches the number of categories in `pdf`. -bool matches(const RooJSONFactoryWSTool::CombinedData &data, const RooSimultaneous *pdf) -{ - return data.components.size() == pdf->indexCat().size(); -} - // True if the entire string parses as a number (integer or floating-point). bool isNumber(const std::string &str) { @@ -955,7 +950,8 @@ void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWo } bool found = false; for (const auto &d : datasets) { - if (d->GetName() == dataRef->name) { + // Components of combined data sets have been moved out of the list. + if (d && d->GetName() == dataRef->name) { found = true; observables.add(*d->get(), true); } @@ -1132,6 +1128,100 @@ void combineDatasets(HS3Document const &doc, std::vector> &datasets) +{ + for (HS3::Data const &input : doc.data) { + auto const *info = std::get_if(&input.body); + if (!info) { + continue; + } + std::string const &name = input.name; + RooJSONFactoryWSTool::testValidName(name, true); + std::string const context = "the combined data set '" + name + "'"; + std::vector columns; + for (std::string const &column : info->columns) { + RooCategory *cat = workspace.cat(column); + if (!cat) { + RooJSONFactoryWSTool::error("the column '" + column + "' of " + context + + " is not declared by a categorical axis of a domain"); + } + columns.push_back(cat); + } + + struct Component { + std::vector const *key; + std::unique_ptr data; + }; + std::vector components; + for (HS3::CombinedDataComponent const &c : info->components) { + for (std::size_t i = 0; i < columns.size(); ++i) { + if (!columns[i]->hasLabel(c.key[i])) { + RooJSONFactoryWSTool::error("the label '" + c.key[i] + "' in position " + std::to_string(i) + + " of a key of " + context + " is not one of the categories of '" + + columns[i]->GetName() + "'"); + } + } + auto found = + std::find_if(datasets.begin(), datasets.end(), [&](auto &d) { return d && d->GetName() == c.data.name; }); + if (found == datasets.end()) { + RooJSONFactoryWSTool::error("the component '" + c.data.name + "' of " + context + + " is not a data set of the document, or it is already a component of " + + "another combined data set"); + } + components.push_back({&c.key, std::move(*found)}); + } + + // The variables: the columns first, then those of the components. A weight variable if any component is + // weighted, with errors if any component is binned, like RooDataSet does when importing a map of data sets. + RooArgSet columnClones; // owns the clones; `vars` must not own anything, it also lists the components' variables + for (RooCategory const *column : columns) { + columnClones.addClone(*column); + } + RooArgSet vars{columnClones}; + bool weighted = false; + bool storeErrors = false; + std::string weightName = RooFit::WeightVar().getString(0); + for (auto const &c : components) { + vars.add(*c.data->get(), /*silent=*/true); + weighted |= c.data->isWeighted(); + if (auto *ds = dynamic_cast(c.data.get()); ds && ds->isWeighted()) { + weightName = ds->weightVar()->GetName(); + } + storeErrors |= dynamic_cast(c.data.get()) != nullptr; + } + std::unique_ptr combined; + if (weighted) { + RooRealVar weightVar{weightName.c_str(), weightName.c_str(), 1.0}; + vars.add(weightVar); + RooArgSet errorSet{weightVar}; + combined = std::make_unique(name.c_str(), name.c_str(), vars, RooFit::WeightVar(weightVar), + storeErrors ? RooFit::StoreError(errorSet) : RooCmdArg::none()); + } else { + combined = std::make_unique(name.c_str(), name.c_str(), vars); + } + for (auto const &c : components) { + for (std::size_t i = 0; i < columns.size(); ++i) { + static_cast(vars[columns[i]->GetName()]).setLabel((*c.key)[i].c_str()); + } + for (int i = 0; i < c.data->numEntries(); ++i) { + vars.assign(*c.data->get(i)); + if (weighted) { + combined->add(vars, c.data->weight(), storeErrors ? c.data->weightError(RooAbsData::SumW2) : 0.0); + } else { + combined->add(vars); + } + } + } + datasets.emplace_back(std::move(combined)); + } +} + template void sortByName(T &coll) { @@ -1155,22 +1245,6 @@ bool isTopLevel(RooAbsArg const &arg, RooWorkspace const &ws) return true; } -/// Find the single category observable of a dataset, if any. -RooAbsCategory *findCategoryObservable(RooAbsData const &data) -{ - RooAbsCategory *cat = nullptr; - for (RooAbsArg *obs : *data.get()) { - if (auto *c = dynamic_cast(obs)) { - if (cat) { - RooJSONFactoryWSTool::error("dataset '" + std::string(data.GetName()) + - " has several category observables!"); - } - cat = c; - } - } - return cat; -} - } // namespace RooArgSet RooFit::HS3::createObservables(std::vector const &axes) @@ -1876,86 +1950,98 @@ void RooJSONFactoryWSTool::exportArray(std::size_t n, double const *contents, JS } /** - * @brief Export a RooAbsCategory object to a JSONNode. - * - * This function exports a RooAbsCategory object, represented by the provided categories and indices, - * to a JSONNode. The category labels and corresponding indices are added to the JSONNode as key-value pairs. + * @brief Export a data set with category observables as a "combined" data set. * - * @param cat The RooAbsCategory object to be exported. - * @param node The JSONNode to which the category data will be exported. - * @return void + * The categories are the columns, sorted by name, and are declared as categorical axes of the default domain. There + * is one sub-data set per key, i.e. per combination of states, written like any other data set and named after the + * data set and the labels of the key. A key is written also without entries: RooFit treats a channel without entries + * as present and empty. If the workspace has a RooSimultaneous with the same columns, each sub-data set only carries + * the observables of the corresponding component, and keys without entries for which it has no component are left + * out; otherwise, every sub-data set carries all observables. */ -void RooJSONFactoryWSTool::exportCategory(RooAbsCategory const &cat, JSONNode &node) -{ - auto &labels = node["labels"].set_seq(); - auto &indices = node["indices"].set_seq(); - - for (auto const &item : cat) { - labels.append_child() << makeValidNameOrError(item.first); - indices.append_child() << item.second; - } -} - -// Split `data` by its index category into per-channel datasets and export each, returning the resulting -// component-name map. RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooAbsData const &data) { - RooAbsCategory *cat = findCategoryObservable(data); - - // prepare return value - RooJSONFactoryWSTool::CombinedData datamap; - - if (!cat) - return datamap; - // this is a combined dataset - - datamap.name = data.GetName(); - - // Write information necessary to reconstruct the combined dataset upon import - HS3::CombinedDataset &info = _export->doc.rooFit.combinedDatasets[data.GetName()]; - info.indexCat = cat->GetName(); - for (auto const &item : *cat) { - info.labels.push_back(makeValidNameOrError(item.first)); - info.indices.push_back(item.second); + std::vector columns = selectionColumns(*data.get()); + CombinedData info; + info.name = data.GetName(); + for (RooCategory const *column : columns) { + info.columns.push_back(column->GetName()); + recordCategory(*column); } - // Find a RooSimultaneous model that would fit to this dataset + // Split the data by the key: by the index category of a matching RooSimultaneous, or by the columns themselves. RooSimultaneous const *simPdf = nullptr; std::vector candidates; for (RooAbsArg *arg : _workspace.allPdfs()) { - auto *candidate = dynamic_cast(arg); - if (candidate && std::string(candidate->indexCat().GetName()) == cat->GetName()) { + if (auto *candidate = dynamic_cast(arg)) { candidates.push_back(candidate); } } - if (!candidates.empty()) { - sortByName(candidates); - simPdf = candidates.front(); + sortByName(candidates); + for (RooSimultaneous const *candidate : candidates) { + std::vector candidateColumns; + for (RooCategory const *column : selectionColumns(candidate->indexCat())) { + candidateColumns.push_back(column->GetName()); + } + if (candidateColumns == info.columns) { + simPdf = candidate; + break; + } + } + std::unique_ptr superCat; + RooAbsCategory const *indexCat = nullptr; + std::vector> parts; + if (simPdf) { + indexCat = &simPdf->indexCat(); + parts = data.split(*simPdf, true); + } else if (columns.size() == 1) { + indexCat = columns.front(); + parts = data.split(*indexCat, true); + } else { + RooArgSet inputs; + for (RooCategory const *column : columns) { + inputs.add(*column); + } + const std::string superName = info.name + "_key"; + superCat = std::make_unique(superName.c_str(), superName.c_str(), inputs); + indexCat = superCat.get(); + parts = data.split(*indexCat, true); } - // If there is an associated simultaneous pdf for the index category, we - // use the RooAbsData::split() overload that takes the RooSimultaneous. - // Like this, the observables that are not relevant for a given channel - // are automatically split from the component datasets. - std::vector> dataList{simPdf ? data.split(*simPdf, true) : data.split(*cat, true)}; - - for (std::unique_ptr const &absData : dataList) { - std::string catName(absData->GetName()); - std::string dataName = makeValidNameOrError(catName); - absData->SetName((std::string(data.GetName()) + "_" + dataName).c_str()); - datamap.components[catName] = absData->GetName(); - this->exportData(*absData); + // The combined data set is written before its sub-data sets. + HS3::CombinedData combined; + combined.columns = info.columns; + std::vector subDataSets; + for (std::vector const &key : selectionKeys(columns)) { + std::string const label = selectionLabel(*indexCat, columns, key); + auto found = std::find_if(parts.begin(), parts.end(), [&](auto &d) { return d->GetName() == label; }); + if (found == parts.end() || ((*found)->numEntries() == 0 && simPdf && !simPdf->getPdf(label))) { + continue; + } + std::string subName = info.name; + for (std::string const &l : key) { + subName += "_" + makeValidNameOrError(l); + } + (*found)->SetName(subName.c_str()); + combined.components.push_back({key, {subName}}); + subDataSets.push_back(found->get()); + } + _export->doc.data.push_back({info.name, std::move(combined)}); + for (RooAbsData *subData : subDataSets) { + exportData(*subData); } - return datamap; + return info; } // Export a single dataset `data` (binned or unbinned) to the output document. void RooJSONFactoryWSTool::exportData(RooAbsData const &data) { - RooAbsCategory *cat = findCategoryObservable(data); - - if (cat) - return; + for (RooAbsArg const *obs : *data.get()) { + if (dynamic_cast(obs)) { + error("the data set '" + std::string(data.GetName()) + "' has the categorical observable '" + obs->GetName() + + "', which is not a RooCategory, and can't be written as a combined data set"); + } + } HS3::Data output; output.name = data.GetName(); @@ -1992,8 +2078,9 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data) // RooDataSets from binned data to fit HistFactory models. In this case, it // doesn't make sense to export them like an unbinned dataset, because the // coordinates are redundant information with the binning. We only do this - // for 1D data for now. - if (data.isWeighted() && variables.size() == 1) { + // for 1D data for now. A variable without bins can't be binned data: unbinned data sets leave their axes without + // binning, and would otherwise be written as binned once they have no entries. + if (data.isWeighted() && variables.size() == 1 && static_cast(*variables[0]).getBins() > 0) { bool isBinnedData = false; auto &x = static_cast(*variables[0]); std::vector contents; @@ -2145,39 +2232,40 @@ void RooJSONFactoryWSTool::exportModelConfig(RooFit::ModelConfig const &mc, const std::vector &combDataSets, const std::vector &singleDataSets) { + // The analyses are named after the ModelConfig, so that ModelConfigs sharing a pdf and a data set (like an S+B + // and B-only pair) get separate analyses. auto pdf = mc.GetPdf(); auto simpdf = dynamic_cast(pdf); + int founddata = 0; if (simpdf) { - for (std::size_t i = 0; i < std::max(combDataSets.size(), std::size_t(1)); ++i) { - const bool hasdata = i < combDataSets.size(); - if (hasdata && !matches(combDataSets.at(i), simpdf)) - continue; - - std::string analysisName(mc.GetName()); - if (hasdata) - analysisName += "_" + combDataSets[i].name; - - exportSingleModelConfig(mc, analysisName, hasdata ? &combDataSets[i].components : nullptr); + // One analysis per combined data set with the same columns. + std::vector columns; + for (RooCategory const *column : selectionColumns(simpdf->indexCat())) { + columns.push_back(column->GetName()); + } + for (auto const &data : combDataSets) { + if (data.columns == columns) { + exportSingleModelConfig(mc, std::string(mc.GetName()) + "_" + data.name, &data.name); + ++founddata; + } } } else { RooArgSet observables(*mc.GetObservables()); - int founddata = 0; for (auto *data : singleDataSets) { if (observables.equals(*(data->get()))) { - std::map mapping; - mapping[pdf->GetName()] = data->GetName(); - exportSingleModelConfig(mc, std::string(mc.GetName()) + "_" + data->GetName(), &mapping); + const std::string dataName = data->GetName(); + exportSingleModelConfig(mc, std::string(mc.GetName()) + "_" + dataName, &dataName); ++founddata; } } - if (founddata == 0) { - exportSingleModelConfig(mc, mc.GetName(), nullptr); - } + } + if (founddata == 0) { + exportSingleModelConfig(mc, mc.GetName(), nullptr); } } void RooJSONFactoryWSTool::exportSingleModelConfig(RooFit::ModelConfig const &mc, std::string const &analysisName, - std::map const *dataComponents) + std::string const *dataName) { HS3Document &doc = _export->doc; auto pdf = mc.GetPdf(); @@ -2190,18 +2278,8 @@ void RooJSONFactoryWSTool::exportSingleModelConfig(RooFit::ModelConfig const &mc likelihood.name = analysisName + "_likelihood"; analysis.likelihood.name = likelihood.name; - if (dataComponents) { - auto simPdf = dynamic_cast(pdf); - if (simPdf) { - for (auto const &item : simPdf->indexCat()) { - const auto &dataComp = dataComponents->find(item.first); - likelihood.terms.push_back({{simPdf->getPdf(item.first)->GetName()}, HS3::DataRef{dataComp->second}}); - } - } else { - for (auto it : *dataComponents) { - likelihood.terms.push_back({{it.first}, HS3::DataRef{it.second}}); - } - } + if (dataName) { + likelihood.terms.push_back({{pdf->GetName()}, HS3::DataRef{*dataName}}); } else { likelihood.terms.push_back({{pdf->GetName()}, 0.0}); } @@ -2212,14 +2290,25 @@ void RooJSONFactoryWSTool::exportSingleModelConfig(RooFit::ModelConfig const &mc aux.push_back({name}); } }; - // The constraint terms of HistFactory channels that no modifier names are part of the likelihood. - for (auto const &term : likelihood.terms) { - auto found = doc.rooFit.channelConstraints.find(term.distribution.name); + // The constraint terms of HistFactory channels that no modifier names are part of the likelihood. The channels + // can be the components of a combined distribution. + auto addChannelConstraints = [&](std::string const &channel) { + auto found = doc.rooFit.channelConstraints.find(channel); if (found != doc.rooFit.channelConstraints.end()) { for (auto const &ref : found->second) { addAux(ref.name); } } + }; + for (auto const &term : likelihood.terms) { + addChannelConstraints(term.distribution.name); + if (auto const *simPdf = dynamic_cast(_workspace.pdf(term.distribution.name))) { + for (auto const &item : simPdf->indexCat()) { + if (RooAbsPdf const *component = simPdf->getPdf(item.first)) { + addChannelConstraints(component->GetName()); + } + } + } } if (mc.GetExternalConstraints()) { for (const auto &constr : *mc.GetExternalConstraints()) { @@ -2322,21 +2411,17 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) alldata.push_back(d); } sortByName(alldata); - // first, take care of combined datasets + // Data sets with category observables are combined data sets. std::vector singleData; std::vector combData; for (auto &d : alldata) { - auto data = this->exportCombinedData(*d); - if (!data.components.empty()) { - combData.push_back(data); - } else { + if (selectionColumns(*d->get()).empty()) { singleData.push_back(d); + this->exportData(*d); + } else { + combData.push_back(this->exportCombinedData(*d)); } } - // next, take care datasets - for (auto &d : alldata) { - this->exportData(*d); - } // export all ModelConfig objects and attached Pdfs for (TObject *obj : _workspace.allGenericObjects()) { @@ -2608,15 +2693,18 @@ void RooJSONFactoryWSTool::importAllNodes(JSONNode &n) importAttributes(arg, item.second); } - // We delay the import of the data to after combineDatasets(), because it - // might be that some datasets are merged to combined datasets there. In - // that case, we will remove the components from the "datasets" vector so they - // don't get imported. + // The components of combined data sets are moved out of the "datasets" vector when the combined data set is + // assembled, so they don't get imported on their own. The ROOT-internal form of older files (combineDatasets()) + // is handled after the analyses, because their likelihoods refer to the components. std::vector> datasets; datasets.reserve(doc.data.size()); for (auto const &data : doc.data) { - datasets.push_back(loadData(data, _workspace, &_import->ranges)); + // Combined data sets are assembled from the others once all of them are loaded. + if (!std::holds_alternative(data.body)) { + datasets.push_back(loadData(data, _workspace, &_import->ranges)); + } } + assembleCombinedData(doc, _workspace, datasets); // Now, read in analyses and likelihoods if there are any diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index c3312ed4e6a3c..6f4a185748f39 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -5681,3 +5681,255 @@ TEST(RooFitHS3, CombinedDistImportErrors) makeJSON(R"(["sample"])", good, R"({"name": "sample", "min": 0.0, "max": 1.0})"), "the column 'sample' of the combined_dist 'simPdf' is not declared by a categorical axis"); } + +// A data set with a category observable is a "combined" data set (HS3 #119): one sub-data set per state, selected +// by a key, with the category declared as a categorical axis of the default domain. A likelihood pairs the combined +// data set with the combined_dist. The ROOT-internal form of older files is still read. +TEST(RooFitHS3, CombinedData) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws{"ws"}; + ws.factory("Gaussian::model_1(x_1[0, 10], mean[5, 0, 10], sigma_1[0.5, 0.1, 10])"); + ws.factory("Gaussian::model_2(x_2[0, 10], mean, sigma_2[0.8, 0.1, 10])"); + ws.factory("SIMUL::simPdf(channelCat[channel_1=0, channel_2=1], channel_1=model_1, channel_2=model_2)"); + std::unique_ptr d1{ws.pdf("model_1")->generate(*ws.var("x_1"), 20)}; + std::unique_ptr d2{ws.pdf("model_2")->generate(*ws.var("x_2"), 30)}; + std::map parts{{"channel_1", d1.get()}, {"channel_2", d2.get()}}; + RooDataSet obsData{"obsData", + "obsData", + {*ws.var("x_1"), *ws.var("x_2")}, + RooFit::Index(*ws.cat("channelCat")), + RooFit::Import(parts)}; + ws.import(obsData); + RooFit::ModelConfig mc{"ModelConfig"}; + mc.SetWS(ws); + mc.SetPdf("simPdf"); + mc.SetParametersOfInterest("mean"); + mc.SetObservables("x_1,x_2,channelCat"); + ws.import(mc); + + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const &root = tree->rootnode(); + + // Structure: the combined entry, one sub-data set per state with only the observables of its component, the + // domain axis, the likelihood pairing the combined objects. + auto const *combined = RooJSONFactoryWSTool::findNamedChild(root["data"], "obsData"); + ASSERT_NE(combined, nullptr) << json; + EXPECT_EQ((*combined)["type"].val(), "combined"); + ASSERT_EQ((*combined)["columns"].num_children(), 1u); + EXPECT_EQ((*combined)["columns"].child(0).val(), "channelCat"); + EXPECT_FALSE(combined->has_child("axes")); + ASSERT_EQ((*combined)["components"].num_children(), 2u); + EXPECT_EQ((*combined)["components"].child(0)["key"].child(0).val(), "channel_1"); + EXPECT_EQ((*combined)["components"].child(0)["dataset"].val(), "obsData_channel_1"); + EXPECT_EQ((*combined)["components"].child(1)["key"].child(0).val(), "channel_2"); + EXPECT_EQ((*combined)["components"].child(1)["dataset"].val(), "obsData_channel_2"); + for (const char *channel : {"1", "2"}) { + auto const *part = RooJSONFactoryWSTool::findNamedChild(root["data"], std::string("obsData_channel_") + channel); + ASSERT_NE(part, nullptr) << json; + EXPECT_EQ((*part)["type"].val(), "unbinned"); + ASSERT_EQ((*part)["axes"].num_children(), 1u); + EXPECT_EQ((*part)["axes"].child(0)["name"].val(), std::string("x_") + channel); + } + EXPECT_FALSE(root.find("misc", "ROOT_internal", "combined_datasets")) << json; + auto const *domain = RooJSONFactoryWSTool::findNamedChild(root["domains"], "default_domain"); + ASSERT_NE(domain, nullptr); + EXPECT_NE(RooJSONFactoryWSTool::findNamedChild((*domain)["axes"], "channelCat"), nullptr) << json; + auto const *likelihood = RooJSONFactoryWSTool::findNamedChild(root["likelihoods"], "ModelConfig_obsData_likelihood"); + ASSERT_NE(likelihood, nullptr) << json; + ASSERT_EQ((*likelihood)["distributions"].num_children(), 1u); + EXPECT_EQ((*likelihood)["distributions"].child(0).val(), "simPdf"); + ASSERT_EQ((*likelihood)["data"].num_children(), 1u); + EXPECT_EQ((*likelihood)["data"].child(0).val(), "obsData"); + EXPECT_NE(RooJSONFactoryWSTool::findNamedChild(root["analyses"], "ModelConfig_obsData"), nullptr) << json; + + // Round trip: one data set with the category column, the same entries per state, the ModelConfig observes the + // category, and the document is stable. + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(json)); + EXPECT_EQ(ws2.allData().size(), 1u); + RooAbsData *obsData2 = ws2.data("obsData"); + ASSERT_NE(obsData2, nullptr); + EXPECT_EQ(obsData2->numEntries(), 50); + auto *channelCat2 = dynamic_cast(obsData2->get()->find("channelCat")); + ASSERT_NE(channelCat2, nullptr); + auto split2 = obsData2->split(*channelCat2); + ASSERT_EQ(split2.size(), 2u); + for (auto const &part : split2) { + EXPECT_EQ(part->numEntries(), std::string(part->GetName()) == "channel_1" ? 20 : 30) << part->GetName(); + } + auto *mc2 = dynamic_cast(ws2.obj("ModelConfig")); + ASSERT_NE(mc2, nullptr); + EXPECT_EQ(mc2->GetPdf(), ws2.pdf("simPdf")); + EXPECT_NE(mc2->GetObservables()->find("channelCat"), nullptr); + EXPECT_EQ(RooJSONFactoryWSTool{ws2}.exportJSONtoString(), json); + + // Without a RooSimultaneous over the category, the sub-data sets carry all observables. + { + RooWorkspace wsDataOnly; + wsDataOnly.import(obsData); + const std::string dataOnly = RooJSONFactoryWSTool{wsDataOnly}.exportJSONtoString(); + auto dataOnlyTree = RooFit::Detail::JSONTree::create(dataOnly); + auto const *part = RooJSONFactoryWSTool::findNamedChild(dataOnlyTree->rootnode()["data"], "obsData_channel_1"); + ASSERT_NE(part, nullptr) << dataOnly; + EXPECT_EQ((*part)["axes"].num_children(), 2u) << dataOnly; + } + + // Files written by earlier ROOT versions: components named after the data set and the state, the combination + // described in misc.ROOT_internal, and the likelihood listing the components. + RooWorkspace wsLegacy; + ASSERT_TRUE(RooJSONFactoryWSTool{wsLegacy}.importJSONfromString(R"({ + "metadata": {"hs3_version": "0.2"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "x_1", "min": 0.0, "max": 10.0}, {"name": "x_2", "min": 0.0, "max": 10.0}, {"name": "mean", "min": 0.0, "max": 10.0}]}], + "distributions": [ + {"name": "model_1", "type": "gaussian_dist", "x": "x_1", "mean": "mean", "sigma": 0.5}, + {"name": "model_2", "type": "gaussian_dist", "x": "x_2", "mean": "mean", "sigma": 0.8}], + "data": [ + {"name": "obsData_channel_1", "type": "unbinned", "axes": [{"name": "x_1", "min": 0.0, "max": 10.0}], "entries": [[1.0], [2.0]]}, + {"name": "obsData_channel_2", "type": "unbinned", "axes": [{"name": "x_2", "min": 0.0, "max": 10.0}], "entries": [[3.0]]}], + "likelihoods": [{"name": "simPdf_obsData_likelihood", "distributions": ["model_1", "model_2"], + "data": ["obsData_channel_1", "obsData_channel_2"]}], + "analyses": [{"name": "simPdf_obsData", "likelihood": "simPdf_obsData_likelihood", "parameters_of_interest": ["mean"]}], + "misc": {"ROOT_internal": { + "combined_datasets": {"obsData": {"index_cat": "channelCat", "labels": ["channel_1", "channel_2"], "indices": [0, 1]}}, + "combined_distributions": {"simPdf": {"index_cat": "channelCat", "labels": ["channel_1", "channel_2"], + "indices": [0, 1], "distributions": ["model_1", "model_2"]}}, + "ModelConfigs": {"simPdf_obsData": {"mcName": "ModelConfig", "pdfName": "simPdf"}}}} + })")); + EXPECT_EQ(wsLegacy.allData().size(), 1u); + ASSERT_NE(wsLegacy.data("obsData"), nullptr); + EXPECT_EQ(wsLegacy.data("obsData")->numEntries(), 3); + auto *mcLegacy = dynamic_cast(wsLegacy.obj("ModelConfig")); + ASSERT_NE(mcLegacy, nullptr); + EXPECT_EQ(mcLegacy->GetPdf(), wsLegacy.pdf("simPdf")); + const std::string fromLegacy = RooJSONFactoryWSTool{wsLegacy}.exportJSONtoString(); + EXPECT_NE(fromLegacy.find(R"("type":"combined")"), std::string::npos) << fromLegacy; + EXPECT_EQ(fromLegacy.find("combined_datasets"), std::string::npos) << fromLegacy; +} + +// A data set with several category observables has several columns, sorted by name. Keys without entries are +// written if the matching RooSimultaneous has a component for them, and left out otherwise. +TEST(RooFitHS3, CombinedDataSeveralColumns) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooRealVar x{"x", "x", 0.0, 10.0}; + RooCategory region{"region", "region", {{"SR", 0}, {"CR", 1}}}; + RooCategory kind{"kind", "kind", {{"physics", 0}, {"global", 1}}}; + RooGaussian sr{"sr", "sr", x, 3.0, 1.0}; + RooGaussian cr{"cr", "cr", x, 6.0, 1.0}; + RooGaussian srGlobal{"sr_global", "sr_global", x, 5.0, 2.0}; + RooSuperCategory index{"index", "index", RooArgSet{region, kind}}; + RooSimultaneous simPdf{ + "simPdf", "simPdf", {{"{SR;physics}", &sr}, {"{CR;physics}", &cr}, {"{SR;global}", &srGlobal}}, index}; + // Entries per key; none for (physics, CR), and (global, CR) has no component. + const std::map, int> entries{{{"physics", "SR"}, 3}, {{"global", "SR"}, 1}}; + RooDataSet obs{"obs", "obs", {x, region, kind}}; + for (auto const &item : entries) { + kind.setLabel(item.first[0].c_str()); + region.setLabel(item.first[1].c_str()); + for (int i = 0; i < item.second; ++i) { + x.setVal(1.0 + i); + obs.add({x, region, kind}); + } + } + + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + ws.import(obs); + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const *combined = RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["data"], "obs"); + ASSERT_NE(combined, nullptr) << json; + ASSERT_EQ((*combined)["columns"].num_children(), 2u); + EXPECT_EQ((*combined)["columns"].child(0).val(), "kind"); + EXPECT_EQ((*combined)["columns"].child(1).val(), "region"); + std::map, std::string> components; + for (auto const &component : (*combined)["components"].children()) { + components[{component["key"].child(0).val(), component["key"].child(1).val()}] = component["dataset"].val(); + } + const std::map, std::string> expected{{{"physics", "SR"}, "obs_physics_SR"}, + {{"physics", "CR"}, "obs_physics_CR"}, + {{"global", "SR"}, "obs_global_SR"}}; + EXPECT_EQ(components, expected) << json; + auto const *emptyPart = RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["data"], "obs_physics_CR"); + ASSERT_NE(emptyPart, nullptr) << json; + EXPECT_EQ((*emptyPart)["entries"].num_children(), 0u); + + // Round trip: one data set with both category columns and the same entries per key; stable document. + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(json)); + EXPECT_EQ(ws2.allData().size(), 1u); + RooAbsData *obs2 = ws2.data("obs"); + ASSERT_NE(obs2, nullptr); + EXPECT_EQ(RooJSONFactoryWSTool{ws2}.exportJSONtoString(), json); + std::map, int> entries2; + for (int i = 0; i < obs2->numEntries(); ++i) { + RooArgSet const &row = *obs2->get(i); + ++entries2[{static_cast(row["kind"]).getCurrentLabel(), + static_cast(row["region"]).getCurrentLabel()}]; + } + EXPECT_EQ(entries2, entries); + // The RooSimultaneous splits it by its RooSuperCategory, which is rebuilt over the columns. + auto parts = obs2->split(*dynamic_cast(ws2.pdf("simPdf")), /*createEmptyDataSets=*/true); + std::map partEntries; + for (auto const &part : parts) { + partEntries[part->GetName()] = part->numEntries(); + } + const std::map expectedParts{ + {"{physics;SR}", 3}, {"{physics;CR}", 0}, {"{global;SR}", 1}, {"{global;CR}", 0}}; + EXPECT_EQ(partEntries, expectedParts); +} + +TEST(RooFitHS3, CombinedDataImportErrors) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + auto makeJSON = [](std::string const &combined, std::string const &axes = "") { + return R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", + "axes": [{"name": "x", "min": 0.0, "max": 10.0}, {"name": "sample", "categories": ["a", "b"]}]}], + "data": [ + {"name": "da", "type": "unbinned", "axes": [{"name": "x", "min": 0.0, "max": 10.0}], "entries": [[1.0]]}, + {"name": "db", "type": "unbinned", "axes": [{"name": "x", "min": 0.0, "max": 10.0}], "entries": [[2.0], [3.0]]}, + {"name": "obs", "type": "combined", )" + + axes + combined + R"(} + ] + })"; + }; + const std::string good = + R"("columns": ["sample"], "components": [{"key": ["a"], "dataset": "da"}, {"key": ["b"], "dataset": "db"}])"; + { + RooWorkspace ws; + ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString(makeJSON(good))); + EXPECT_EQ(ws.allData().size(), 1u); + ASSERT_NE(ws.data("obs"), nullptr); + EXPECT_EQ(ws.data("obs")->numEntries(), 3); + } + expectImportThrowsWithError(makeJSON(good, R"("axes": [{"name": "x", "min": 0.0, "max": 10.0}], )"), + "the combined data set 'obs' must not have \"axes\""); + expectImportThrowsWithError( + makeJSON(R"("columns": ["channel"], "components": [{"key": ["a"], "dataset": "da"}])"), + "the column 'channel' of the combined data set 'obs' is not declared by a categorical axis"); + expectImportThrowsWithError(makeJSON(R"("columns": ["sample"], "components": [])"), + "\"components\" of the combined data set 'obs' must be a non-empty list"); + expectImportThrowsWithError(makeJSON(R"("columns": ["sample"], "components": [{"key": ["a"]}])"), + "must have a \"key\" and a \"dataset\""); + expectImportThrowsWithError(makeJSON(R"("columns": ["sample"], "components": [{"key": ["c"], "dataset": "da"}])"), + "the label 'c' in position 0 of a key of the combined data set 'obs'"); + expectImportThrowsWithError(makeJSON(R"("columns": ["sample"], "components": [{"key": ["a"], "dataset": "dc"}])"), + "the component 'dc' of the combined data set 'obs' is not a data set of the document"); + expectImportThrowsWithError( + makeJSON( + R"("columns": ["sample"], "components": [{"key": ["a"], "dataset": "da"}, {"key": ["b"], "dataset": "da"}])"), + "the component 'da' of the combined data set 'obs' is not a data set of the document, or it is already a " + "component"); + expectImportThrowsWithError( + makeJSON( + R"("columns": ["sample"], "components": [{"key": ["a"], "dataset": "da"}, {"key": ["a"], "dataset": "db"}])"), + "the key [\"a\"] is used by two components of the combined data set 'obs'"); +} From 127db91fb3d7dad7b3d8c7d14fa97caf152ddc9a Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 16:14:24 +0100 Subject: [PATCH 03/18] [RF][HS3] Check the pairing of combined distributions and data sets HS3 #119 treats a key of a combined distribution without a sub-data set in the paired combined data set as an unobserved component, which is marginalised, and a present but empty sub-data set as observed with zero events. RooFit has no notion of an unobserved component: a combined RooDataSet without entries for a channel is a present and empty channel, and both evaluation backends then add the expected number of events of an extended component to the likelihood (see ROOT issue #24004). When a likelihood pairs a combined_dist with a combined data set, the import now checks the keys. A missing sub-data set of an extended component is an error, since the fit would differ from what the document means. A missing sub-data set of a non-extended component changes nothing and is a warning, as is a sub-data set for a key without a component, which RooFit ignores. Differing columns are an error. Test: CombinedDataPairing. Ported onto the typed HS3 document: the keys of the combined data set are taken from the document. Assisted-by: claude-code:claude-fable-5-1 Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/RooJSONFactoryWSTool.cxx | 84 ++++++++++++++++++++ roofit/hs3/test/testRooFitHS3.cxx | 100 ++++++++++++++++++++++++ 2 files changed, 184 insertions(+) diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 992fea78b772a..f015c1c3d17bb 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -883,6 +883,75 @@ void requirePdf(RooAbsArg const *arg, std::string const &what) } } +// Check that a combined data set paired with a RooSimultaneous in a likelihood has a sub-data set for every component. +// RooFit has no notion of an unobserved component: a key without sub-data set is a channel without entries, which +// adds the expected number of events of an extended component to the likelihood. That changes the fit, so it is an +// error (https://github.com/root-project/root/issues/24004). For a non-extended component it changes nothing, and a +// sub-data set without component is ignored by RooFit; both are reported as warnings. +void checkCombinedPairing(RooSimultaneous const &simPdf, std::string const &dataName, HS3::CombinedData const &data) +{ + std::vector columns = selectionColumns(simPdf.indexCat()); + std::vector columnNames; + for (RooCategory const *column : columns) { + columnNames.push_back(column->GetName()); + } + std::vector dataColumns = data.columns; + std::sort(dataColumns.begin(), dataColumns.end()); + if (dataColumns != columnNames) { + RooJSONFactoryWSTool::error("the likelihood pairs '" + std::string(simPdf.GetName()) + + "' with the combined data set '" + dataName + "', but their columns differ"); + } + // The keys of the data, each as a map from the column to the label. + std::set> dataKeys; + for (HS3::CombinedDataComponent const &component : data.components) { + std::map key; + for (std::size_t i = 0; i < data.columns.size(); ++i) { + key[data.columns[i]] = component.key[i]; + } + dataKeys.insert(key); + } + auto keyToString = [&](std::map const &key) { + std::vector labels; + for (std::string const &column : data.columns) { + labels.push_back(key.at(column)); + } + return selectionKeyToString(labels); + }; + std::set> pdfKeys; + for (std::vector const &labels : selectionKeys(columns)) { + RooAbsPdf const *component = simPdf.getPdf(selectionLabel(simPdf.indexCat(), columns, labels)); + if (!component) { + continue; + } + std::map key; + for (std::size_t i = 0; i < columns.size(); ++i) { + key[columnNames[i]] = labels[i]; + } + pdfKeys.insert(key); + if (dataKeys.count(key) > 0) { + continue; + } + if (component->canBeExtended()) { + RooJSONFactoryWSTool::error( + "the combined data set '" + dataName + "' has no sub-data set for the key " + keyToString(key) + + " of the extended component '" + component->GetName() + "' of '" + simPdf.GetName() + + "'. RooFit cannot leave a component unobserved: it would add the expected number of events of the " + + "component to the likelihood, see https://github.com/root-project/root/issues/24004"); + } + RooJSONFactoryWSTool::warning("RooFitHS3: the combined data set '" + dataName + + "' has no sub-data set for the key " + keyToString(key) + " of the component '" + + component->GetName() + "' of '" + simPdf.GetName() + + "', which RooFit treats as present and empty."); + } + for (auto const &key : dataKeys) { + if (pdfKeys.count(key) == 0) { + RooJSONFactoryWSTool::warning("RooFitHS3: the sub-data set for the key " + keyToString(key) + + " of the combined data set '" + dataName + "' is ignored, because '" + + simPdf.GetName() + "' has no component for it."); + } + } +} + // Import an analysis (likelihood + domains) as one or more ModelConfig objects into the workspace. void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWorkspace &workspace, const std::vector> &datasets) @@ -960,6 +1029,21 @@ void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWo throw std::runtime_error("dataset '" + dataRef->name + "' cannot be found!"); } + // A combined distribution paired with a combined data set must have a sub-data set for every component. + for (auto const &term : likelihood.terms) { + auto const *dataRef = std::get_if(&term.data); + auto *simPdf = dynamic_cast(workspace.pdf(term.distribution.name)); + if (!dataRef || !simPdf) { + continue; + } + for (HS3::Data const &data : doc.data) { + auto const *combined = std::get_if(&data.body); + if (combined && data.name == dataRef->name) { + checkCombinedPairing(*simPdf, data.name, *combined); + } + } + } + std::string const pdfName = mcInfo && mcInfo->pdfName ? *mcInfo->pdfName : "simPdf"; RooAbsPdf *pdf = workspace.pdf(pdfName); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 6f4a185748f39..9b926b370cba4 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -5933,3 +5933,103 @@ TEST(RooFitHS3, CombinedDataImportErrors) R"("columns": ["sample"], "components": [{"key": ["a"], "dataset": "da"}, {"key": ["a"], "dataset": "db"}])"), "the key [\"a\"] is used by two components of the combined data set 'obs'"); } + +// A likelihood that pairs a combined_dist with a combined data set: HS3 treats a key without sub-data set as an +// unobserved component, which RooFit cannot represent (it would be a channel without entries, adding the expected +// events of an extended component to the likelihood). This is an error for extended components and a warning +// otherwise; a sub-data set without component is ignored with a warning. +TEST(RooFitHS3, CombinedDataPairing) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + // `components` of the distribution and of the data, and whether the components are extended. + auto makeJSON = [](std::string const &pdfComponents, std::string const &dataComponents, bool extended) { + auto component = [&](std::string const &label, double mean) { + const std::string gauss = R"({"name": "g_)" + label + R"(", "type": "gaussian_dist", "x": "x_)" + label + + R"(", "mean": )" + std::to_string(mean) + R"(, "sigma": 1.0})"; + if (!extended) { + return gauss; + } + return gauss + R"(, {"name": "e_)" + label + R"(", "type": "rate_extended_dist", "dist": "g_)" + label + + R"(", "rate": "n_)" + label + R"("})"; + }; + auto data = [](std::string const &label) { + return R"({"name": "d_)" + label + R"(", "type": "unbinned", "axes": [{"name": "x_)" + label + + R"(", "min": 0.0, "max": 10.0}], "entries": [[5.0]]})"; + }; + return R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "x_a", "min": 0.0, "max": 10.0}, {"name": "x_b", "min": 0.0, "max": 10.0}, + {"name": "x_c", "min": 0.0, "max": 10.0}, {"name": "n_a", "min": 0.0, "max": 100.0}, + {"name": "n_b", "min": 0.0, "max": 100.0}, {"name": "n_c", "min": 0.0, "max": 100.0}, + {"name": "sample", "categories": ["a", "b", "c"]}]}], + "distributions": [)" + + component("a", 3.0) + ", " + component("b", 4.0) + ", " + component("c", 5.0) + R"(, + {"name": "simPdf", "type": "combined_dist", "columns": ["sample"], "components": )" + + pdfComponents + R"(}], + "data": [)" + + data("a") + ", " + data("b") + ", " + data("c") + R"(, + {"name": "obs", "type": "combined", "columns": ["sample"], "components": )" + + dataComponents + R"(}], + "likelihoods": [{"name": "nll", "distributions": ["simPdf"], "data": ["obs"]}], + "analyses": [{"name": "fit", "likelihood": "nll", "domains": ["default_domain"]}] + })"; + }; + auto pdfComponents = [](std::string const &prefix, std::vector const &labels) { + std::string out = "["; + for (auto const &label : labels) { + out += std::string(out.size() > 1 ? ", " : "") + R"({"key": [")" + label + R"("], "dist": ")" + prefix + + label + "\"}"; + } + return out + "]"; + }; + auto dataComponents = [](std::vector const &labels) { + std::string out = "["; + for (auto const &label : labels) { + out += + std::string(out.size() > 1 ? ", " : "") + R"({"key": [")" + label + R"("], "dataset": "d_)" + label + "\"}"; + } + return out + "]"; + }; + + // Every component has a sub-data set: no messages. + { + RooWorkspace ws; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString( + makeJSON(pdfComponents("e_", {"a", "b", "c"}), dataComponents({"a", "b", "c"}), true))); + }); + EXPECT_EQ(warnings, ""); + auto *mc = dynamic_cast(ws.obj("fit")); + ASSERT_NE(mc, nullptr); + EXPECT_EQ(mc->GetPdf(), ws.pdf("simPdf")); + } + // An extended component without sub-data set is an error. + expectImportThrowsWithError(makeJSON(pdfComponents("e_", {"a", "b", "c"}), dataComponents({"a", "b"}), true), + "the combined data set 'obs' has no sub-data set for the key [\"c\"] of the extended " + "component 'e_c' of 'simPdf'"); + // A non-extended component without sub-data set is a warning, and so is a sub-data set without component. + { + RooWorkspace ws; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString( + makeJSON(pdfComponents("g_", {"a", "b", "c"}), dataComponents({"a", "b"}), false))); + }); + EXPECT_NE(warnings.find("the combined data set 'obs' has no sub-data set for the key [\"c\"] of the component " + "'g_c' of 'simPdf', which RooFit treats as present and empty"), + std::string::npos) + << warnings; + } + { + RooWorkspace ws; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString( + makeJSON(pdfComponents("e_", {"a", "b"}), dataComponents({"a", "b", "c"}), true))); + }); + EXPECT_NE(warnings.find("the sub-data set for the key [\"c\"] of the combined data set 'obs' is ignored, " + "because 'simPdf' has no component for it"), + std::string::npos) + << warnings; + } +} From 9aca3158638a7e9b65b87a00d29abdc6c3307c98 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 16:23:37 +0100 Subject: [PATCH 04/18] [RF][HS3] Test that HistFactory models are written with combined objects A HistFactory model is the standard case of a RooSimultaneous with combined data sets. HS3ClosureLoop now also checks that it is written as a combined_dist with combined data sets, without the ROOT_internal combined_distributions and combined_datasets, and read back into a RooSimultaneous over channelCat and data sets with the index category, with the same number of entries and the same sum of weights. The stability of the document and the fit results were already checked. Assisted-by: claude-code:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/test/testRooFitHS3.cxx | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 9b926b370cba4..ce5cc9546bc8d 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -4382,11 +4382,31 @@ TEST_P(HFFixture, HS3ClosureLoop) // Chack that JSON > WS > JSON doesn't change the JSON EXPECT_EQ(js, js3) << "The JSON -> WS -> JSON roundtrip did not result in the original JSON!"; - auto *newmc = dynamic_cast(wsFromJson.obj("ModelConfig")); - EXPECT_TRUE(newmc != nullptr); + // The RooSimultaneous and its data sets are the standard case of combined objects (HS3 #119): they are written + // as a "combined_dist" and "combined" data sets, and read back into a RooSimultaneous and data sets with the + // index category. + EXPECT_NE(js.find(R"("type":"combined_dist")"), std::string::npos) << js; + EXPECT_NE(js.find(R"("type":"combined")"), std::string::npos) << js; + EXPECT_EQ(js.find("combined_datasets"), std::string::npos) << js; + EXPECT_EQ(js.find("combined_distributions"), std::string::npos) << js; - RooAbsPdf *newpdf = newmc->GetPdf(); - EXPECT_TRUE(newpdf != nullptr); + auto *newmc = dynamic_cast(wsFromJson.obj("ModelConfig")); + ASSERT_NE(newmc, nullptr); + + auto *newpdf = dynamic_cast(newmc->GetPdf()); + ASSERT_NE(newpdf, nullptr); + EXPECT_STREQ(newpdf->indexCat().GetName(), "channelCat"); + EXPECT_NE(newmc->GetObservables()->find("channelCat"), nullptr); + EXPECT_EQ(wsFromJson.allData().size(), ws->allData().size()); + for (const char *dataName : {"obsData", "asimovData"}) { + RooAbsData *origData = ws->data(dataName); + RooAbsData *readData = wsFromJson.data(dataName); + ASSERT_NE(origData, nullptr) << dataName; + ASSERT_NE(readData, nullptr) << dataName; + EXPECT_EQ(readData->numEntries(), origData->numEntries()) << dataName; + EXPECT_DOUBLE_EQ(readData->sumEntries(), origData->sumEntries()) << dataName; + EXPECT_NE(readData->get()->find("channelCat"), nullptr) << dataName; + } RooAbsData *data = ws->data("obsData"); EXPECT_TRUE(data != nullptr); From 3e032d355dd0d8f43a8b333de3334c335e1bd69e Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 16:29:06 +0100 Subject: [PATCH 05/18] [RF][HS3] Test that combined objects keep their names without misc The names of a RooSimultaneous and of a combined data set used to live in misc.ROOT_internal, so a document without "misc" lost them. Now they are part of the combined_dist and combined entries. The test exports a workspace with a RooSimultaneous (not named "simPdf", the historical default), removes "misc", imports the document and checks that the pdf, its components, the index category and the data set keep their names, that the ModelConfig finds its pdf through the likelihood, and that the re-export describes the same objects. Assisted-by: claude-code:claude-fable-5-1 Co-Authored-By: Claude Fable 5.1 --- roofit/hs3/test/testRooFitHS3.cxx | 92 +++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index ce5cc9546bc8d..4fd8b338b8322 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6053,3 +6053,95 @@ TEST(RooFitHS3, CombinedDataPairing) << warnings; } } + +// The names of a RooSimultaneous and of a combined data set are part of the standard document now, not of +// misc.ROOT_internal: a document without "misc" imports with the same names, and the ModelConfig finds its pdf +// through the likelihood. +TEST(RooFitHS3, CombinedObjectsKeepNamesWithoutMisc) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + // Remove the top-level key `key` (and the comma before it) from a compact JSON document. + auto removeTopLevelKey = [](std::string json, std::string const &key) { + const std::string needle = ",\"" + key + "\":"; + const std::size_t begin = json.find(needle); + if (begin == std::string::npos) { + return json; + } + std::size_t pos = begin + needle.size(); + int depth = 0; + bool inString = false; + for (; pos < json.size(); ++pos) { + const char c = json[pos]; + if (inString) { + if (c == '\\') { + ++pos; + } else if (c == '"') { + inString = false; + } + } else if (c == '"') { + inString = true; + } else if (c == '{' || c == '[') { + ++depth; + } else if (c == '}' || c == ']') { + if (--depth == 0) { + ++pos; + break; + } + } + } + return json.erase(begin, pos - begin); + }; + + RooWorkspace ws{"ws"}; + ws.factory("Gaussian::model_1(x_1[0, 10], mean[5, 0, 10], sigma_1[0.5, 0.1, 10])"); + ws.factory("Gaussian::model_2(x_2[0, 10], mean, sigma_2[0.8, 0.1, 10])"); + // Not named "simPdf", which older documents relied on as the default name. + ws.factory("SIMUL::channels(region[SR=0, CR=1], SR=model_1, CR=model_2)"); + std::unique_ptr d1{ws.pdf("model_1")->generate(*ws.var("x_1"), 20)}; + std::unique_ptr d2{ws.pdf("model_2")->generate(*ws.var("x_2"), 30)}; + std::map parts{{"SR", d1.get()}, {"CR", d2.get()}}; + RooDataSet observed{"observed", + "observed", + {*ws.var("x_1"), *ws.var("x_2")}, + RooFit::Index(*ws.cat("region")), + RooFit::Import(parts)}; + ws.import(observed); + RooFit::ModelConfig mc{"ModelConfig"}; + mc.SetWS(ws); + mc.SetPdf("channels"); + mc.SetParametersOfInterest("mean"); + mc.SetObservables("x_1,x_2,region"); + ws.import(mc); + + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + const std::string withoutMisc = removeTopLevelKey(json, "misc"); + ASSERT_NE(withoutMisc, json); + EXPECT_EQ(withoutMisc.find("\"misc\""), std::string::npos) << withoutMisc; + + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(withoutMisc)); + auto *channels = dynamic_cast(ws2.pdf("channels")); + ASSERT_NE(channels, nullptr); + EXPECT_STREQ(channels->indexCat().GetName(), "region"); + EXPECT_STREQ(channels->getPdf("SR")->GetName(), "model_1"); + EXPECT_STREQ(channels->getPdf("CR")->GetName(), "model_2"); + EXPECT_EQ(ws2.allData().size(), 1u); + RooAbsData *observed2 = ws2.data("observed"); + ASSERT_NE(observed2, nullptr); + EXPECT_EQ(observed2->numEntries(), 50); + EXPECT_NE(observed2->get()->find("region"), nullptr); + // Without misc.ROOT_internal.ModelConfigs, the ModelConfig is named after the analysis. + auto *mc2 = dynamic_cast(ws2.obj("ModelConfig_observed")); + ASSERT_NE(mc2, nullptr); + EXPECT_EQ(mc2->GetPdf(), channels); + EXPECT_NE(mc2->GetObservables()->find("region"), nullptr); + EXPECT_NE(mc2->GetParametersOfInterest()->find("mean"), nullptr); + + // The analysis is named after the ModelConfig, which got the name of the analysis: the names of the analysis and + // the likelihood change once on the re-export. From then on, the document is stable. + const std::string json2 = RooJSONFactoryWSTool{ws2}.exportJSONtoString(); + RooWorkspace ws3; + ASSERT_TRUE(RooJSONFactoryWSTool{ws3}.importJSONfromString(json2)); + EXPECT_EQ(RooJSONFactoryWSTool{ws3}.exportJSONtoString(), json2); +} From 7858de402fa9979a8b524233959baa7a4e757160 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 16:50:52 +0100 Subject: [PATCH 06/18] [RF][HS3] Reject combined distributions whose components share an observable HS3 PR #119 requires that the components of a combined distribution share no variate, and that the sub-data sets of a combined data set have distinct axis names: a quantity measured in several components is a separate random variable in each of them, e.g. "mass_SR" and "mass_CR", since it follows a different distribution there. A RooSimultaneous whose components use the same observable can therefore not be written together with its data set. Exporting such a data set, split by the components of the RooSimultaneous, is now an error that asks to give the observable a separate name in each component. A document whose sub-data sets share an axis is still read, with a warning. The test of a data set with several columns now uses a separate observable per component. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/RooJSONFactoryWSTool.cxx | 49 ++++++++++++++++ roofit/hs3/test/testRooFitHS3.cxx | 74 +++++++++++++++++++++---- 2 files changed, 113 insertions(+), 10 deletions(-) diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index f015c1c3d17bb..dcf7a57dec949 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -1212,6 +1212,34 @@ void combineDatasets(HS3Document const &doc, std::vector +std::vector sharedVariableNames(DataSets const &datasets, GetData getData) +{ + std::map counts; + for (auto const &item : datasets) { + for (RooAbsArg const *var : *getData(item)->get()) { + ++counts[var->GetName()]; + } + } + std::vector shared; + for (auto const &[name, count] : counts) { + if (count > 1) { + shared.push_back(name); + } + } + return shared; +} + +std::string joinQuoted(std::vector const &names) +{ + std::string out; + for (std::string const &name : names) { + out += (out.empty() ? "'" : ", '") + name + "'"; + } + return out; +} + // Build the combined data sets of the document: a RooDataSet with one category column per entry of "columns" and the // variables of all components, filled row by row from the components, which are then no longer imported on their own. // The columns are categories declared by categorical axes of the domains. A key without a sub-data set is a state @@ -1260,6 +1288,15 @@ void assembleCombinedData(HS3Document const &doc, RooWorkspace const &workspace, } components.push_back({&c.key, std::move(*found)}); } + // HS3 requires distinct axis names, since the components of a combined distribution share no variate. RooFit + // can still read such a document. + std::vector shared = sharedVariableNames(components, [](auto const &c) { return c.data.get(); }); + if (!shared.empty()) { + RooJSONFactoryWSTool::warning("RooFitHS3: the sub-data sets of " + context + " share the axes " + + joinQuoted(shared) + + ". HS3 requires distinct axis names, because the components of a combined " + "distribution share no variate. RooFit reads the document anyway."); + } // The variables: the columns first, then those of the components. A weight variable if any component is // weighted, with errors if any component is binned, like RooDataSet does when importing a map of data sets. @@ -2110,6 +2147,18 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA combined.components.push_back({key, {subName}}); subDataSets.push_back(found->get()); } + // In HS3, the components of a combined distribution share no variate: a quantity measured in several components is a + // separate variate in each of them, e.g. "mass_SR" and "mass_CR". The sub-data sets, which are split by the + // components of the RooSimultaneous, must therefore not share observables. + if (simPdf) { + std::vector shared = sharedVariableNames(subDataSets, [](RooAbsData const *d) { return d; }); + if (!shared.empty()) { + error("the data set '" + info.name + "' is split by the components of '" + simPdf->GetName() + + "', which share the observables " + joinQuoted(shared) + + ". HS3 requires each component of a combined distribution to have its own variates: give the " + "observable a separate name in each component (e.g. 'mass_SR' and 'mass_CR' instead of 'mass')."); + } + } _export->doc.data.push_back({info.name, std::move(combined)}); for (RooAbsData *subData : subDataSets) { exportData(*subData); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 4fd8b338b8322..c9a897d5d6edb 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -3704,17 +3704,18 @@ TEST(RooFitHS3, ModelConfigsSharingPdfAndData) } { RooWorkspace ws; - ws.factory("Gaussian::pdf_a(x[0, -5, 5], mu[0, -5, 5], sigma[1, 0.1, 5])"); - ws.factory("Gaussian::pdf_b(x, mu, sigma)"); + // Each component has its own observable, as HS3 requires. + ws.factory("Gaussian::pdf_a(x_a[0, -5, 5], mu[0, -5, 5], sigma[1, 0.1, 5])"); + ws.factory("Gaussian::pdf_b(x_b[0, -5, 5], mu, sigma)"); ws.factory("SIMUL::sim_pdf(channel[a=0, b=1], a=pdf_a, b=pdf_b)"); - RooArgSet observables{*ws.var("x"), *ws.cat("channel")}; + RooArgSet observables{*ws.var("x_a"), *ws.var("x_b"), *ws.cat("channel")}; RooDataSet data{"data", "data", observables}; for (int index : {0, 1}) { ws.cat("channel")->setIndex(index); data.add(observables); } ws.import(data, RooFit::Silence()); - roundTrip(ws, "sim_pdf", "x,channel"); + roundTrip(ws, "sim_pdf", "x_a,x_b,channel"); } } @@ -5836,24 +5837,28 @@ TEST(RooFitHS3, CombinedDataSeveralColumns) { RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); - RooRealVar x{"x", "x", 0.0, 10.0}; + // Each component has its own observable, as HS3 requires. + RooRealVar xSR{"x_sr", "x_sr", 0.0, 10.0}; + RooRealVar xCR{"x_cr", "x_cr", 0.0, 10.0}; + RooRealVar xGlobal{"x_sr_global", "x_sr_global", 0.0, 10.0}; RooCategory region{"region", "region", {{"SR", 0}, {"CR", 1}}}; RooCategory kind{"kind", "kind", {{"physics", 0}, {"global", 1}}}; - RooGaussian sr{"sr", "sr", x, 3.0, 1.0}; - RooGaussian cr{"cr", "cr", x, 6.0, 1.0}; - RooGaussian srGlobal{"sr_global", "sr_global", x, 5.0, 2.0}; + RooGaussian sr{"sr", "sr", xSR, 3.0, 1.0}; + RooGaussian cr{"cr", "cr", xCR, 6.0, 1.0}; + RooGaussian srGlobal{"sr_global", "sr_global", xGlobal, 5.0, 2.0}; RooSuperCategory index{"index", "index", RooArgSet{region, kind}}; RooSimultaneous simPdf{ "simPdf", "simPdf", {{"{SR;physics}", &sr}, {"{CR;physics}", &cr}, {"{SR;global}", &srGlobal}}, index}; // Entries per key; none for (physics, CR), and (global, CR) has no component. const std::map, int> entries{{{"physics", "SR"}, 3}, {{"global", "SR"}, 1}}; - RooDataSet obs{"obs", "obs", {x, region, kind}}; + RooDataSet obs{"obs", "obs", {xSR, xCR, xGlobal, region, kind}}; for (auto const &item : entries) { kind.setLabel(item.first[0].c_str()); region.setLabel(item.first[1].c_str()); + RooRealVar &x = item.first[0] == "global" ? xGlobal : xSR; for (int i = 0; i < item.second; ++i) { x.setVal(1.0 + i); - obs.add({x, region, kind}); + obs.add({xSR, xCR, xGlobal, region, kind}); } } @@ -6145,3 +6150,52 @@ TEST(RooFitHS3, CombinedObjectsKeepNamesWithoutMisc) ASSERT_TRUE(RooJSONFactoryWSTool{ws3}.importJSONfromString(json2)); EXPECT_EQ(RooJSONFactoryWSTool{ws3}.exportJSONtoString(), json2); } + +// In HS3, the components of a combined distribution share no variate: a quantity measured in several components is a +// separate variate in each of them ("mass_SR", "mass_CR"). A RooSimultaneous whose components share an observable can +// therefore not be written with its data set. A document whose sub-data sets share an axis is still read, with a +// warning. +TEST(RooFitHS3, CombinedComponentsMustNotShareObservables) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooRealVar mass{"mass", "mass", 0.0, 10.0}; + RooCategory channel{"channel", "channel", {{"SR", 0}, {"CR", 1}}}; + RooGaussian sr{"sr", "sr", mass, 3.0, 1.0}; + RooGaussian cr{"cr", "cr", mass, 6.0, 1.0}; + RooSimultaneous simPdf{"simPdf", "simPdf", {{"SR", &sr}, {"CR", &cr}}, channel}; + RooDataSet obs{"obs", "obs", {mass, channel}}; + for (const char *label : {"SR", "CR"}) { + channel.setLabel(label); + mass.setVal(4.0); + obs.add({mass, channel}); + } + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + ws.import(obs); + const std::string errors = captureMessages(RooFit::ERROR, RooFit::IO, [&] { + EXPECT_THROW(RooJSONFactoryWSTool{ws}.exportJSONtoString(), std::runtime_error); + }); + EXPECT_NE(errors.find("which share the observables 'mass'"), std::string::npos) << errors; + EXPECT_NE(errors.find("'mass_SR' and 'mass_CR'"), std::string::npos) << errors; + + const std::string json = R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "channel", "categories": ["SR", "CR"]}, {"name": "mass", "min": 0.0, "max": 10.0}]}], + "data": [ + {"name": "obs_SR", "type": "unbinned", "axes": [{"name": "mass", "min": 0.0, "max": 10.0}], "entries": [[4.0]]}, + {"name": "obs_CR", "type": "unbinned", "axes": [{"name": "mass", "min": 0.0, "max": 10.0}], "entries": [[5.0]]}, + {"name": "obs", "type": "combined", "columns": ["channel"], + "components": [{"key": ["SR"], "dataset": "obs_SR"}, {"key": ["CR"], "dataset": "obs_CR"}]} + ] + })"; + RooWorkspace imported; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + EXPECT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + }); + EXPECT_NE(warnings.find("share the axes 'mass'"), std::string::npos) << warnings; + RooAbsData *data = imported.data("obs"); + ASSERT_NE(data, nullptr); + EXPECT_EQ(data->numEntries(), 2); +} From 2776da027e6199b50e33f5bdde4c70cec874ada1 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 17:43:48 +0100 Subject: [PATCH 07/18] [RF][HS3] Write data sets with categories as categorical axes without a combined distribution A combined data set is paired with a combined distribution, and HS3 PR #119 requires its sub-data sets to have distinct axis names. A data set with a category observable but without a matching RooSimultaneous was split by the category anyway, so all sub-data sets carried the same observables. Without a matching RooSimultaneous, the category is an ordinary observable: such a data set is now written as a single data set with a categorical axis (HS3 PR #108), binned or unbinned, which RooFit reads back since the axis handling PR. The categories are declared by a categorical axis of the default domain, and the entries of an unbinned categorical axis are the labels. Ported onto the typed HS3 document: the categorical axes are written through the HS3::CategoricalAxis of binned and unbinned data, with the position of the label as the entry of unbinned data. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 2 + roofit/hs3/src/RooJSONFactoryWSTool.cxx | 132 ++++++++++++++---- roofit/hs3/test/testRooFitHS3.cxx | 27 +++- 3 files changed, 126 insertions(+), 35 deletions(-) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index e1ae803268089..ac7d173716a8b 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -47,6 +47,7 @@ class ModelConfig; } class RooRealVar; class RooAbsCategory; +class RooCategory; class RooSimultaneous; class RooJSONFactoryWSTool { @@ -201,6 +202,7 @@ class RooJSONFactoryWSTool { } static void exportAxis(RooFit::Detail::JSONNode &obsNode, RooRealVar const &var); + static void exportCategoricalAxis(RooFit::Detail::JSONNode &obsNode, RooCategory const &cat); static void exportHisto(RooArgSet const &vars, std::size_t n, double const *contents, RooFit::Detail::JSONNode &output); diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index dcf7a57dec949..2f097496e267c 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -1635,6 +1635,49 @@ std::string makeValidNameOrError(std::string const &in) return out; } +// The labels of a selection key, made valid names and joined by "_". Used in the names of sub-data sets and of +// renamed observables. +std::string selectionKeyName(std::vector const &key) +{ + std::string out; + for (std::string const &label : key) { + out += (out.empty() ? "" : "_") + makeValidNameOrError(label); + } + return out; +} + +// The names of the category columns of `vars`. +std::vector selectionColumnNames(RooAbsCollection const &vars) +{ + std::vector names; + for (RooCategory const *column : selectionColumns(vars)) { + names.push_back(column->GetName()); + } + return names; +} + +// The RooSimultaneous of the workspace (the first by name) whose columns are `columns`, or nullptr. +RooSimultaneous const *findMatchingSimultaneous(RooWorkspace const &workspace, std::vector const &columns) +{ + std::vector candidates; + for (RooAbsArg *arg : workspace.allPdfs()) { + if (auto *candidate = dynamic_cast(arg)) { + candidates.push_back(candidate); + } + } + sortByName(candidates); + for (RooSimultaneous const *candidate : candidates) { + std::vector candidateColumns; + for (RooCategory const *column : selectionColumns(candidate->indexCat())) { + candidateColumns.push_back(column->GetName()); + } + if (candidateColumns == columns) { + return candidate; + } + } + return nullptr; +} + } // namespace namespace { @@ -2013,6 +2056,18 @@ void RooJSONFactoryWSTool::importFunction(const std::string &jsonString, bool im * @param var The RooRealVar representing the axis to be exported. * @return void */ +// A categorical axis: the labels in the order of the states of the category, which is the order of the bins of a +// RooDataHist. +void RooJSONFactoryWSTool::exportCategoricalAxis(JSONNode &obsNode, RooCategory const &cat) +{ + testValidName(cat.GetName(), false); + obsNode["name"] << cat.GetName(); + auto &labels = obsNode["categories"].set_seq(); + for (unsigned int i = 0; i < cat.size(); ++i) { + labels.append_child() << cat.getOrdinal(i).first; + } +} + void RooJSONFactoryWSTool::exportAxis(JSONNode &obsNode, RooRealVar const &var) { std::string name = var.GetName(); @@ -2038,8 +2093,12 @@ void RooJSONFactoryWSTool::exportHisto(RooArgSet const &vars, std::size_t n, dou { auto &observablesNode = output["axes"].set_seq(); // axes have to be ordered to get consistent bin indices - for (auto *var : static_range_cast(vars)) { - exportAxis(observablesNode.append_child().set_map(), *var); + for (RooAbsArg const *var : vars) { + if (auto const *cat = dynamic_cast(var)) { + exportCategoricalAxis(observablesNode.append_child().set_map(), *cat); + } else { + exportAxis(observablesNode.append_child().set_map(), static_cast(*var)); + } } return exportArray(n, contents, output["contents"]); @@ -2091,24 +2150,7 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA } // Split the data by the key: by the index category of a matching RooSimultaneous, or by the columns themselves. - RooSimultaneous const *simPdf = nullptr; - std::vector candidates; - for (RooAbsArg *arg : _workspace.allPdfs()) { - if (auto *candidate = dynamic_cast(arg)) { - candidates.push_back(candidate); - } - } - sortByName(candidates); - for (RooSimultaneous const *candidate : candidates) { - std::vector candidateColumns; - for (RooCategory const *column : selectionColumns(candidate->indexCat())) { - candidateColumns.push_back(column->GetName()); - } - if (candidateColumns == info.columns) { - simPdf = candidate; - break; - } - } + RooSimultaneous const *simPdf = findMatchingSimultaneous(_workspace, info.columns); std::unique_ptr superCat; RooAbsCategory const *indexCat = nullptr; std::vector> parts; @@ -2139,10 +2181,7 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA if (found == parts.end() || ((*found)->numEntries() == 0 && simPdf && !simPdf->getPdf(label))) { continue; } - std::string subName = info.name; - for (std::string const &l : key) { - subName += "_" + makeValidNameOrError(l); - } + const std::string subName = info.name + "_" + selectionKeyName(key); (*found)->SetName(subName.c_str()); combined.components.push_back({key, {subName}}); subDataSets.push_back(found->get()); @@ -2170,7 +2209,7 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA void RooJSONFactoryWSTool::exportData(RooAbsData const &data) { for (RooAbsArg const *obs : *data.get()) { - if (dynamic_cast(obs)) { + if (dynamic_cast(obs) && !dynamic_cast(obs)) { error("the data set '" + std::string(data.GetName()) + "' has the categorical observable '" + obs->GetName() + "', which is not a RooCategory, and can't be written as a combined data set"); } @@ -2190,9 +2229,26 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data) variables.remove(*weightVar); } + // Categories are categorical axes, declared by a categorical axis of the default domain. + auto categoricalAxis = [&](RooCategory const &cat) { + recordCategory(cat); + HS3::CategoricalAxis axis; + axis.name = cat.GetName(); + testValidName(axis.name, false); + for (unsigned int i = 0; i < cat.size(); ++i) { + axis.categories.push_back(cat.getOrdinal(i).first); + } + return axis; + }; + auto exportBinned = [&](std::size_t n, double const *contents) { HS3::BinnedData binned; - for (auto *var : static_range_cast(variables)) { + for (RooAbsArg const *arg : variables) { + if (auto const *cat = dynamic_cast(arg)) { + binned.axes.emplace_back(categoricalAxis(*cat)); + continue; + } + auto const *var = static_cast(arg); recordDomains(_export->domains, *var); binned.axes.emplace_back(HS3::toBinnedAxis(*var)); } @@ -2213,7 +2269,8 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data) // coordinates are redundant information with the binning. We only do this // for 1D data for now. A variable without bins can't be binned data: unbinned data sets leave their axes without // binning, and would otherwise be written as binned once they have no entries. - if (data.isWeighted() && variables.size() == 1 && static_cast(*variables[0]).getBins() > 0) { + if (data.isWeighted() && variables.size() == 1 && dynamic_cast(variables[0]) && + static_cast(*variables[0]).getBins() > 0) { bool isBinnedData = false; auto &x = static_cast(*variables[0]); std::vector contents; @@ -2233,7 +2290,12 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data) // this really is an unbinned dataset HS3::UnbinnedData unbinned; - for (auto *var : static_range_cast(variables)) { + for (RooAbsArg const *arg : variables) { + if (auto const *cat = dynamic_cast(arg)) { + unbinned.axes.emplace_back(categoricalAxis(*cat)); + continue; + } + auto const *var = static_cast(arg); recordDomains(_export->domains, *var); // HS3 forbids binning information on the axes of unbinned data. testValidName(var->GetName(), false); @@ -2243,8 +2305,13 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data) bool hasNonUnityWeights = false; for (int i = 0; i < data.numEntries(); ++i) { data.get(i); - for (auto *var : static_range_cast(variables)) { - unbinned.entries.push_back(var->getVal()); + // The entries of a categorical axis are the positions of the labels in its "categories". + for (RooAbsArg const *arg : variables) { + if (auto const *cat = dynamic_cast(arg)) { + unbinned.entries.push_back(cat->getCurrentOrdinalNumber()); + } else { + unbinned.entries.push_back(static_cast(arg)->getVal()); + } } if (data.isWeighted()) { weightVals.push_back(data.weight()); @@ -2548,7 +2615,10 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) std::vector singleData; std::vector combData; for (auto &d : alldata) { - if (selectionColumns(*d->get()).empty()) { + // Without a matching RooSimultaneous, the categories of a data set are ordinary observables, written as + // categorical axes. + if (selectionColumns(*d->get()).empty() || + !findMatchingSimultaneous(_workspace, selectionColumnNames(*d->get()))) { singleData.push_back(d); this->exportData(*d); } else { diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index c9a897d5d6edb..fd9e049c88f8f 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -5787,15 +5787,34 @@ TEST(RooFitHS3, CombinedData) EXPECT_NE(mc2->GetObservables()->find("channelCat"), nullptr); EXPECT_EQ(RooJSONFactoryWSTool{ws2}.exportJSONtoString(), json); - // Without a RooSimultaneous over the category, the sub-data sets carry all observables. + // Without a RooSimultaneous over the category, the category is an ordinary observable: the data set is written + // with a categorical axis (HS3 combined data sets require a combined distribution to pair with), and read back + // with the same entries per state. { RooWorkspace wsDataOnly; wsDataOnly.import(obsData); const std::string dataOnly = RooJSONFactoryWSTool{wsDataOnly}.exportJSONtoString(); auto dataOnlyTree = RooFit::Detail::JSONTree::create(dataOnly); - auto const *part = RooJSONFactoryWSTool::findNamedChild(dataOnlyTree->rootnode()["data"], "obsData_channel_1"); - ASSERT_NE(part, nullptr) << dataOnly; - EXPECT_EQ((*part)["axes"].num_children(), 2u) << dataOnly; + auto const &dataNode = dataOnlyTree->rootnode()["data"]; + EXPECT_EQ(RooJSONFactoryWSTool::findNamedChild(dataNode, "obsData_channel_1"), nullptr) << dataOnly; + auto const *single = RooJSONFactoryWSTool::findNamedChild(dataNode, "obsData"); + ASSERT_NE(single, nullptr) << dataOnly; + EXPECT_EQ((*single)["type"].val(), "unbinned") << dataOnly; + auto const *channelAxis = RooJSONFactoryWSTool::findNamedChild((*single)["axes"], "channelCat"); + ASSERT_NE(channelAxis, nullptr) << dataOnly; + EXPECT_EQ((*channelAxis)["categories"].num_children(), 2u) << dataOnly; + + RooWorkspace wsDataOnly2; + ASSERT_TRUE(RooJSONFactoryWSTool{wsDataOnly2}.importJSONfromString(dataOnly)); + RooAbsData *read = wsDataOnly2.data("obsData"); + ASSERT_NE(read, nullptr); + EXPECT_EQ(read->numEntries(), 50); + auto split = read->split(*static_cast(read->get()->find("channelCat"))); + ASSERT_EQ(split.size(), 2u); + for (auto const &part : split) { + EXPECT_EQ(part->numEntries(), std::string(part->GetName()) == "channel_1" ? 20 : 30) << part->GetName(); + } + EXPECT_EQ(RooJSONFactoryWSTool{wsDataOnly2}.exportJSONtoString(), dataOnly); } // Files written by earlier ROOT versions: components named after the data set and the state, the combination From 1ad626162726800822c50918066bb87ef68c38c2 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 17:43:48 +0100 Subject: [PATCH 08/18] [RF][HS3] Optionally rename the observables of combined data sets per key HS3 PR #119 requires that the components of a combined distribution share no variate. Workspaces whose RooSimultaneous components share an observable (common in older workspaces and in CMS Combine) can therefore not be exported. The new option RooJSONFactoryWSTool::config() .renameCombinedObservables (off by default) renames the observables of every combined data set per key on export: "obs_" (the labels of the key joined by "_"), or "obs__" if a component has several observables. The components of the matching RooSimultaneous are written with the renamed observables: they are rebuilt with RooCustomizer, which clones every node that depends on a renamed observable with the suffix "_", so nodes shared by several components get one copy per component. Observables that already have their new name are left alone, so that a second export of an imported document gives the same document. The likelihood is unchanged. Ported onto the typed HS3 document. The data set of the test is called "data" instead of "obs": its sub-data sets "obs_SR" and "obs_CR" had the names of the renamed observables, which the validation of HS3 0.3 documents now rejects. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 14 ++ roofit/hs3/src/RooJSONFactoryWSTool.cxx | 125 +++++++++++++++++- roofit/hs3/test/testRooFitHS3.cxx | 92 +++++++++++++ 3 files changed, 226 insertions(+), 5 deletions(-) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index ac7d173716a8b..5416e4de515b3 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -62,6 +62,13 @@ class RooJSONFactoryWSTool { /// before ROOT 6.26 by their structure, and don't write them, since `histfactory_dist` implies them. Turn this /// off if a workspace has such factors that are not meant as bin widths: they are then written as normfactors. bool identifyLegacyBinWidthFactors = true; + /// HS3 requires the components of a combined distribution to have distinct variates, e.g. "mass_SR" and + /// "mass_CR" instead of one "mass". If enabled, the observables of every combined data set are renamed on + /// export to "obs_" (the labels of the key joined by "_"), or "obs__" if a component + /// has several observables; the components of the matching RooSimultaneous are written with the renamed + /// observables (functions depending on them get the suffix "_"). This allows exporting workspaces whose + /// components share an observable. Off by default: without it, such workspaces can't be exported. + bool renameCombinedObservables = false; }; static Config &config(); @@ -281,6 +288,13 @@ class RooJSONFactoryWSTool { std::unique_ptr _export; std::vector _serversToExport; std::vector _serversToDelete; + // Renaming of the observables of combined data sets, see Config::renameCombinedObservables: for each data set and + // each selection label, the new name of each observable; for each RooSimultaneous and selection label, the + // component with renamed observables to export instead of the original one. The objects are owned here. + std::map>> _combinedObservableRenames; + std::map> _renamedComponents; + std::vector> _renamedObjects; + void prepareCombinedObservableRenaming(); // The objects that are currently being imported, innermost last, to detect circular dependencies. std::vector _importStack; diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 2f097496e267c..f9dfd7b7af373 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -1858,21 +1859,41 @@ void RooJSONFactoryWSTool::exportSimultaneous(RooSimultaneous const &simPdf, std recordCategory(*column); } auto &components = elem["components"].set_seq(); + // Components with renamed observables replace the original ones (Config::renameCombinedObservables). + auto renamed = _renamedComponents.find(simPdf.GetName()); + std::set replaced; + std::vector replacements; // In the order of the columns and of their states, which is the order in which the categories are written to // the domain. This does not depend on the order of the inputs of a RooSuperCategory, so the document is the same // after a round trip. for (std::vector const &key : selectionKeys(columns)) { - RooAbsPdf const *pdf = simPdf.getPdf(selectionLabel(indexCat, columns, key)); + const std::string label = selectionLabel(indexCat, columns, key); + RooAbsPdf const *pdf = simPdf.getPdf(label); if (!pdf) { continue; } + if (renamed != _renamedComponents.end()) { + auto found = renamed->second.find(label); + if (found != renamed->second.end()) { + replaced.insert(pdf); + pdf = found->second; + replacements.push_back(pdf); + } + } JSONNode &component = components.append_child().set_map(); component["key"].fill_seq(key); component["dist"] << pdf->GetName(); } recordGenericObject(_export->doc, true, simPdf.GetName(), elem); - exportObjects(simPdf.servers(), exportedObjectNames); + std::vector servers; + for (RooAbsArg const *server : simPdf.servers()) { + if (replaced.count(server) == 0) { + servers.push_back(server); + } + } + servers.insert(servers.end(), replacements.begin(), replacements.end()); + exportObjects(servers, exportedObjectNames); } // Declare a category as a categorical axis of the "default_domain". The states are written in their insertion order: @@ -2129,6 +2150,87 @@ void RooJSONFactoryWSTool::exportArray(std::size_t n, double const *contents, JS } } +// Config::renameCombinedObservables: work out, before anything is exported, the new names of the observables of +// every combined data set, and the components of the matching RooSimultaneous with these observables. One +// observable per key is named "obs_", several are named "obs__". Observables that already +// have their new name are left alone, so that a second export of an imported document gives the same result. +void RooJSONFactoryWSTool::prepareCombinedObservableRenaming() +{ + _combinedObservableRenames.clear(); + _renamedComponents.clear(); + _renamedObjects.clear(); + if (!config().renameCombinedObservables) { + return; + } + std::vector datasets; + for (RooAbsData *data : _workspace.allData()) { + datasets.push_back(data); + } + sortByName(datasets); + for (RooAbsData const *data : datasets) { + std::vector columns = selectionColumns(*data->get()); + if (columns.empty()) { + continue; + } + RooSimultaneous const *simPdf = findMatchingSimultaneous(_workspace, selectionColumnNames(*data->get())); + for (std::vector const &key : selectionKeys(columns)) { + const std::string keyName = selectionKeyName(key); + RooArgSet candidates; + RooAbsPdf const *component = nullptr; + std::string label; + if (simPdf) { + label = selectionLabel(simPdf->indexCat(), columns, key); + component = simPdf->getPdf(label); + if (!component) { + continue; + } + std::unique_ptr observables{component->getObservables(*data->get())}; + candidates.add(*observables); + } else { + candidates.add(*data->get()); + } + std::vector observables; + for (RooAbsArg *arg : candidates) { + if (!dynamic_cast(arg)) { + observables.push_back(arg); + } + } + sortByName(observables); + std::map renames; + for (RooAbsArg const *obs : observables) { + std::string const newName = + "obs_" + keyName + (observables.size() == 1 ? "" : "_" + std::string(obs->GetName())); + if (newName == obs->GetName()) { + continue; + } + if (_workspace.arg(newName.c_str())) { + error("can't rename the observable '" + std::string(obs->GetName()) + "' of the data set '" + + data->GetName() + "' to '" + newName + "': an object of that name already exists"); + } + renames[obs->GetName()] = newName; + } + if (renames.empty()) { + continue; + } + _combinedObservableRenames[data->GetName()][keyName] = renames; + if (!simPdf || _renamedComponents[simPdf->GetName()].count(label) > 0) { + continue; + } + // The component with the renamed observables. Nodes that depend on them are cloned with the suffix + // "_", so that nodes shared by several components get one copy per component. + RooCustomizer customizer{*component, keyName.c_str()}; + for (auto const &[from, to] : renames) { + RooAbsArg *original = candidates.find(from.c_str()); + _renamedObjects.emplace_back(static_cast(original->clone(to.c_str()))); + customizer.replaceArg(*original, *_renamedObjects.back()); + } + auto *renamedComponent = static_cast(customizer.build()); + _renamedObjects.emplace_back(renamedComponent); + _renamedComponents[simPdf->GetName()][label] = renamedComponent; + } + } +} + /** * @brief Export a data set with category observables as a "combined" data set. * @@ -2181,8 +2283,19 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA if (found == parts.end() || ((*found)->numEntries() == 0 && simPdf && !simPdf->getPdf(label))) { continue; } - const std::string subName = info.name + "_" + selectionKeyName(key); + const std::string keyName = selectionKeyName(key); + const std::string subName = info.name + "_" + keyName; (*found)->SetName(subName.c_str()); + // Config::renameCombinedObservables: give the observables of this key their own names. + auto renamesOfData = _combinedObservableRenames.find(info.name); + if (renamesOfData != _combinedObservableRenames.end()) { + auto renames = renamesOfData->second.find(keyName); + if (renames != renamesOfData->second.end()) { + for (auto const &[from, to] : renames->second) { + (*found)->changeObservableName(from.c_str(), to.c_str()); + } + } + } combined.components.push_back({key, {subName}}); subDataSets.push_back(found->get()); } @@ -2574,6 +2687,7 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) doc.metadata.hs3Version = hs3VersionTag; doc.metadata.packages.push_back({"ROOT", rootVersion()}); doc.parameterPoints.push_back({"default_values", {}}); + prepareCombinedObservableRenaming(); // export all toplevel pdfs std::vector allpdfs; @@ -2616,9 +2730,10 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) std::vector combData; for (auto &d : alldata) { // Without a matching RooSimultaneous, the categories of a data set are ordinary observables, written as - // categorical axes. + // categorical axes, unless its observables are renamed per key (Config::renameCombinedObservables). if (selectionColumns(*d->get()).empty() || - !findMatchingSimultaneous(_workspace, selectionColumnNames(*d->get()))) { + (!config().renameCombinedObservables && + !findMatchingSimultaneous(_workspace, selectionColumnNames(*d->get())))) { singleData.push_back(d); this->exportData(*d); } else { diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index fd9e049c88f8f..0cf4317610af3 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6218,3 +6218,95 @@ TEST(RooFitHS3, CombinedComponentsMustNotShareObservables) ASSERT_NE(data, nullptr); EXPECT_EQ(data->numEntries(), 2); } + +// With RooJSONFactoryWSTool::config().renameCombinedObservables, the observables of combined data sets are renamed +// per key on export ("obs_SR", or "obs_SR_x" with several observables), and the components of the matching +// RooSimultaneous are written with them; nodes that depend on a renamed observable get one copy per component. This +// allows exporting a RooSimultaneous whose components share an observable. The likelihood stays the same, and a +// second export gives the same document. +TEST(RooFitHS3, RenameCombinedObservables) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + struct FlagGuard { + bool old = RooJSONFactoryWSTool::config().renameCombinedObservables; + FlagGuard() { RooJSONFactoryWSTool::config().renameCombinedObservables = true; } + ~FlagGuard() { RooJSONFactoryWSTool::config().renameCombinedObservables = old; } + } flagGuard; + + RooRealVar mass{"mass", "mass", 0.0, 10.0}; + RooCategory channel{"channel", "channel", {{"SR", 0}, {"CR", 1}}}; + // A function of the observable, shared by both components. + RooFormulaVar shift{"shift", "mass - 1.0", {mass}}; + RooGaussian sr{"sr", "sr", shift, 3.0, 1.0}; + RooGaussian cr{"cr", "cr", shift, 6.0, 1.5}; + RooSimultaneous simPdf{"simPdf", "simPdf", {{"SR", &sr}, {"CR", &cr}}, channel}; + RooDataSet obs{"data", "data", {mass, channel}}; + for (auto const &[label, value] : + std::vector>{{"SR", 3.5}, {"SR", 4.5}, {"CR", 6.0}, {"CR", 7.5}, {"CR", 8.0}}) { + channel.setLabel(label); + mass.setVal(value); + obs.add({mass, channel}); + } + std::unique_ptr nll{simPdf.createNLL(obs)}; + const double expectedNll = nll->getVal(); + + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + ws.import(obs); + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const &root = tree->rootnode(); + + // The components and the functions of the observable, one per key, with the renamed observables. + auto const *combined = RooJSONFactoryWSTool::findNamedChild(root["distributions"], "simPdf"); + ASSERT_NE(combined, nullptr) << json; + std::map components; + for (auto const &component : (*combined)["components"].children()) { + components[component["key"].child(0).val()] = component["dist"].val(); + } + EXPECT_EQ(components, (std::map{{"SR", "sr_SR"}, {"CR", "cr_CR"}})) << json; + for (const char *key : {"SR", "CR"}) { + auto const *func = RooJSONFactoryWSTool::findNamedChild(root["functions"], std::string("shift_") + key); + ASSERT_NE(func, nullptr) << json; + EXPECT_NE((*func)["expression"].val().find(std::string("obs_") + key), std::string::npos) << json; + auto const *part = RooJSONFactoryWSTool::findNamedChild(root["data"], std::string("data_") + key); + ASSERT_NE(part, nullptr) << json; + ASSERT_EQ((*part)["axes"].num_children(), 1u) << json; + EXPECT_EQ((*part)["axes"].child(0)["name"].val(), std::string("obs_") + key) << json; + } + EXPECT_EQ(RooJSONFactoryWSTool::findNamedChild(root["functions"], "shift"), nullptr) << json; + + // Read back, the likelihood is the same. + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + auto *simPdf2 = dynamic_cast(imported.pdf("simPdf")); + ASSERT_NE(simPdf2, nullptr); + RooAbsData *obs2 = imported.data("data"); + ASSERT_NE(obs2, nullptr); + std::unique_ptr nll2{simPdf2->createNLL(*obs2)}; + EXPECT_NEAR(nll2->getVal(), expectedNll, 1e-10); + + // The observables already have their new names, so a second export gives the same document. + EXPECT_EQ(RooJSONFactoryWSTool{imported}.exportJSONtoString(), json); + + // Several observables per key: "obs__". Without a RooSimultaneous, all observables of the data + // set are renamed. + RooRealVar x{"x", "x", 0.0, 1.0}; + RooRealVar y{"y", "y", 0.0, 1.0}; + RooDataSet xy{"xy", "xy", {x, y, channel}}; + for (const char *label : {"SR", "CR"}) { + channel.setLabel(label); + xy.add({x, y, channel}); + } + RooWorkspace wsXY; + wsXY.import(xy); + const std::string jsonXY = RooJSONFactoryWSTool{wsXY}.exportJSONtoString(); + auto treeXY = RooFit::Detail::JSONTree::create(jsonXY); + auto const *partSR = RooJSONFactoryWSTool::findNamedChild(treeXY->rootnode()["data"], "xy_SR"); + ASSERT_NE(partSR, nullptr) << jsonXY; + std::set axes; + for (auto const &axis : (*partSR)["axes"].children()) { + axes.insert(axis["name"].val()); + } + EXPECT_EQ(axes, (std::set{"obs_SR_x", "obs_SR_y"})) << jsonXY; +} From b6ef39b450d7fa5309291f2815ad2c07ea6f23af Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Fri, 6 Nov 2026 09:12:42 +0100 Subject: [PATCH 09/18] [RF][HS3] Reject "extended" on a combined_dist HS3 PR #119 now states that a combined_dist must not have the component "extended": whether it is extended follows from its components. The importer ignored such a component; it now raises an error, whatever its value. RooFit never writes it. Assisted-by: claude-code:claude-opus-5-5 --- roofit/hs3/src/JSONFactories_RooFitCore.cxx | 5 +++++ roofit/hs3/test/testRooFitHS3.cxx | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index 66702515d67cb..4f7dfe008ac4a 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -428,6 +428,11 @@ bool importCombinedDist(RooJSONFactoryWSTool *tool, const JSONNode &p) { std::string const name(RooJSONFactoryWSTool::name(p)); std::string const context = "the combined_dist '" + name + "'"; + // Whether a combined distribution is extended follows from its components; HS3 forbids declaring it. + if (p.has_child("extended")) { + RooJSONFactoryWSTool::error(context + " must not have \"extended\": whether it is extended follows from its " + + "components"); + } std::vector columns = readSelectionColumns(*tool->workspace(), p, "columns", context); auto const *componentsNode = p.find("components"); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 0cf4317610af3..ed899ff82d49a 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -5697,6 +5697,10 @@ TEST(RooFitHS3, CombinedDistImportErrors) expectImportThrowsWithError(makeJSON(R"(["sample"])", R"([])"), "\"components\" of the combined_dist 'simPdf' must be a non-empty list"); expectImportThrowsWithError(makeJSON(R"(["sample"])", R"([{"key": ["a"]}])"), "must have a \"key\" and a \"dist\""); + // Whether a combined distribution is extended follows from its components: declaring it is not allowed, whatever + // the value. + expectImportThrowsWithError(makeJSON(R"(["sample"])", good + R"(, "extended": false)"), + "the combined_dist 'simPdf' must not have \"extended\""); // The column must be categorical. expectImportThrowsWithError( makeJSON(R"(["sample"])", good, R"({"name": "sample", "min": 0.0, "max": 1.0})"), From b8816f45de38cfefa9fab754dbb90ce16469e288 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Tue, 10 Nov 2026 17:26:17 +0100 Subject: [PATCH 10/18] [RF][HS3] Distinct variates of likelihood terms and combined components HS3 requires the components of a combined distribution to have distinct variates, and the terms of a likelihood as well. A variate in two of them is one quantity modelled twice, e.g. a constraint that is both a term of the likelihood and a factor of a channel, or two quantities with one name, e.g. the observable "mass" of two channels, which need their own names ("mass_SR", "mass_CR"). All cases are now checked by one function, sharedVariates(), on the document, after the export and before the import: - the terms of each likelihood: its distributions (the components of a combined_dist, or of a RooSimultaneous in ROOT_internal of older documents, are terms of their own), its "aux_distributions", and the constraint terms named by the modifiers of its histfactory_dist terms; - the components of the combined distributions and of the combined data sets that no likelihood lists. The variates of a distribution are the "x" of the fundamental distributions and the axes of distributions with axes, through products, mixtures and combined distributions; those of a data set are its axes. This replaces the two checks on the sub-data sets of combined data sets: the export error (only with a data set) and the import warning. - Export: a constraint that is both a factor of a channel's product and a term of the likelihood (a HistFactory constraint, or an external constraint of the ModelConfig) is removed from the product, so that it is written once; RooFit counts it once. Only products that no other distribution uses are changed. Remaining shared variates are an error, now also for a RooSimultaneous without data set. The error mentions config().renameCombinedObservables, and the new config().allowExportSharedVariates, which writes the document anyway, with a warning for every shared variate. It is not valid HS3, and has to be fixed, e.g. by hand in the JSON, before it can be read back. - Import: shared variates are an error for HS3 0.3, and a warning for documents of older versions, which RooFit wrote that way. Tests: - CombinedComponentsMustNotShareObservables: the export error, the override with warning, the rejection of the written document, and of a combined data set with a shared axis (read with a warning as 0.2). - SharedVariatesInLikelihood: two channels with the observable "x". - ConstraintInModelAndExternal: a constraint in the model and in the external constraints is written once, reads back with the same NLL. - SimultaneousGaussians, CombinedDist, CombinedDistSeveralColumns, CombinedDistImportErrors and CombinedDataImportErrors give the components their own observables. Ported onto the typed HS3 document: the check works on the JSON tree as before. On export, it runs after writeDocument(), since the likelihoods and the data sets are only in the JSON tree from then on; on import, it runs on the upgraded document. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 6 + roofit/hs3/src/RooJSONFactoryWSTool.cxx | 440 ++++++++++++++++-- roofit/hs3/test/testRooFitHS3.cxx | 172 ++++++- 3 files changed, 548 insertions(+), 70 deletions(-) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index 5416e4de515b3..5f52793695d9d 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -69,6 +69,12 @@ class RooJSONFactoryWSTool { /// observables (functions depending on them get the suffix "_"). This allows exporting workspaces whose /// components share an observable. Off by default: without it, such workspaces can't be exported. bool renameCombinedObservables = false; + /// HS3 requires the terms of a likelihood and the components of a combined distribution to have distinct + /// variates, e.g. "mass_SR" and "mass_CR" for the observables of two channels instead of one "mass". By + /// default, the export fails otherwise. If true, the document is written anyway, with a warning for every + /// shared variate. It is not valid HS3 and can't be read back before the variates are renamed, e.g. by hand in + /// the JSON. + bool allowExportSharedVariates = false; }; static Config &config(); diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index f9dfd7b7af373..abe3d4be43842 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -39,6 +39,7 @@ #include #include +#include #include #include #include @@ -613,6 +614,359 @@ bool isLiteralConstVar(RooAbsArg const &arg) return isRooConstVar && isNumber(arg.GetName()); } +// --------------------------------------------------------------------------------------------------------------- +// Distinct variates. HS3 requires the terms of a likelihood, and the components of a combined distribution or data +// set, to have distinct variates: a variate in two of them is one quantity modelled twice (e.g. a constraint that is +// a term of the likelihood and a factor of a channel), or two quantities with one name (e.g. the observable "mass" +// of two channels, which need their own names, "mass_SR" and "mass_CR"). All cases are checked with the same +// function, sharedVariates(), on the document, after the export and before the import. + +bool isName(JSONNode const &node) +{ + return !node.is_container() && !node.is_null() && !node.is_number(); +} + +// Add the names in `node` (one name or a list) to `out`. +void addNames(JSONNode const &node, std::set &out) +{ + if (node.is_seq()) { + for (JSONNode const &child : node.children()) { + addNames(child, out); + } + } else if (isName(node)) { + out.insert(node.val()); + } +} + +// The child `name` of the section `section` of the document, or nullptr. +JSONNode const *findInSection(JSONNode const &rootnode, const char *section, std::string const &name) +{ + JSONNode const *node = rootnode.find(section); + return node ? RooJSONFactoryWSTool::findNamedChild(*node, name) : nullptr; +} + +std::string typeOf(JSONNode const &node) +{ + return node.has_child("type") ? node["type"].val() : ""; +} + +// The names of the components of a combined distribution or data set, written as {"key": ..., : }. +std::vector combinedComponents(JSONNode const &node, const char *field) +{ + std::vector out; + if (JSONNode const *components = node.find("components")) { + for (JSONNode const &component : components->children()) { + if (JSONNode const *name = component.find(field); name && isName(*name)) { + out.push_back(name->val()); + } + } + } + return out; +} + +// The variates of the distribution `name` of the document: the "x" of a fundamental distribution (a name or a list) +// and the axes of a distribution with axes (e.g. histfactory_dist), through products, mixtures and combined +// distributions. The constraint terms named by a histfactory_dist are terms of their own, see likelihoodTerms(). +void collectVariates(JSONNode const &rootnode, std::string const &name, std::set &out, + std::set &visited) +{ + if (!visited.insert(name).second) { + return; + } + JSONNode const *node = findInSection(rootnode, "distributions", name); + if (!node) { + return; + } + std::string const type = typeOf(*node); + auto recurse = [&](std::vector const &parts) { + for (std::string const &part : parts) { + collectVariates(rootnode, part, out, visited); + } + }; + auto namesIn = [&](const char *key) { + std::vector parts; + if (JSONNode const *list = node->find(key)) { + for (JSONNode const &part : list->children()) { + if (isName(part)) { + parts.push_back(part.val()); + } + } + } + return parts; + }; + if (type == "product_dist") { + recurse(namesIn("factors")); + } else if (type == "mixture_dist") { + recurse(namesIn("summands")); + } else if (type == "combined_dist") { + recurse(combinedComponents(*node, "dist")); + } else { + if (JSONNode const *x = node->find("x")) { + addNames(*x, out); + } + if (JSONNode const *axes = node->find("axes")) { + for (JSONNode const &axis : axes->children()) { + if (JSONNode const *axisName = axis.find("name")) { + addNames(*axisName, out); + } + } + } + } +} + +std::set variatesOf(JSONNode const &rootnode, std::string const &name) +{ + std::set variates; + std::set visited; + collectVariates(rootnode, name, variates, visited); + return variates; +} + +// The terms of the likelihood `likelihood` of the document, each once: its distributions (the components of a +// combined distribution, or of a RooSimultaneous written to ROOT_internal by older versions, are terms of their own), +// its "aux_distributions", and the constraint terms named by the modifiers of its histfactory_dist terms. +std::vector likelihoodTerms(JSONNode const &rootnode, JSONNode const &likelihood) +{ + std::vector terms; + auto add = [&](std::string const &term) { + if (std::find(terms.begin(), terms.end(), term) == terms.end()) { + terms.push_back(term); + } + }; + for (const char *key : {"distributions", "aux_distributions"}) { + JSONNode const *list = likelihood.find(key); + if (!list) { + continue; + } + for (JSONNode const &item : list->children()) { + if (!isName(item)) { + continue; + } + std::string const name = item.val(); + if (JSONNode const *node = findInSection(rootnode, "distributions", name)) { + if (typeOf(*node) == "combined_dist") { + for (std::string const &component : combinedComponents(*node, "dist")) { + add(component); + } + continue; + } + } else if (JSONNode const *legacy = findRooFitInternal(rootnode, "combined_distributions", name)) { + if (JSONNode const *components = legacy->find("distributions")) { + for (JSONNode const &component : components->children()) { + add(component.val()); + } + continue; + } + } + add(name); + } + } + for (std::size_t i = 0; i < terms.size(); ++i) { + JSONNode const *node = findInSection(rootnode, "distributions", terms[i]); + if (!node || typeOf(*node) != "histfactory_dist" || !node->has_child("samples")) { + continue; + } + for (JSONNode const &sample : (*node)["samples"].children()) { + if (JSONNode const *modifiers = sample.find("modifiers")) { + for (JSONNode const &modifier : modifiers->children()) { + if (JSONNode const *constraints = modifier.find("constraints")) { + std::set names; + addNames(*constraints, names); + for (std::string const &constraint : names) { + add(constraint); + } + } + } + } + } + } + return terms; +} + +// The parts (terms or components) of something that must have distinct variates, with their variates. +using PartVariates = std::vector>>; + +// The variates that several of `parts` share, one message each. `what` names the parts and `context` the whole, +// e.g. "terms" of "the likelihood 'lh'". +std::vector sharedVariates(PartVariates const &parts, std::string const &what, std::string const &context) +{ + std::map> partsOfVariate; + std::vector order; + for (auto const &[part, variates] : parts) { + for (std::string const &variate : variates) { + auto &list = partsOfVariate[variate]; + if (list.empty()) { + order.push_back(variate); + } + if (std::find(list.begin(), list.end(), part) == list.end()) { + list.push_back(part); + } + } + } + std::vector messages; + for (std::string const &variate : order) { + auto const &list = partsOfVariate[variate]; + if (list.size() < 2) { + continue; + } + std::string names; + for (std::size_t i = 0; i < list.size(); ++i) { + names += (i == 0 ? "'" : (i + 1 == list.size() ? "' and '" : "', '")) + list[i]; + } + messages.push_back("the variate '" + variate + "' is shared by the " + what + " " + names + "' of " + context); + } + return messages; +} + +// All shared variates of the document: of the terms of each likelihood, and of the components of the combined +// distributions and combined data sets that no likelihood lists (the others are checked through the likelihood). +std::vector sharedVariatesOfDocument(JSONNode const &rootnode) +{ + std::vector messages; + auto append = [&](std::vector const &more) { + messages.insert(messages.end(), more.begin(), more.end()); + }; + std::set inLikelihoods; + if (JSONNode const *likelihoods = rootnode.find("likelihoods")) { + for (JSONNode const &likelihood : likelihoods->children()) { + for (const char *key : {"distributions", "data", "aux_distributions"}) { + if (JSONNode const *list = likelihood.find(key)) { + for (JSONNode const &item : list->children()) { + inLikelihoods.insert(item.val()); + } + } + } + PartVariates parts; + for (std::string const &term : likelihoodTerms(rootnode, likelihood)) { + parts.emplace_back(term, variatesOf(rootnode, term)); + } + append(sharedVariates(parts, "terms", "the likelihood '" + RooJSONFactoryWSTool::name(likelihood) + "'")); + } + } + if (JSONNode const *distributions = rootnode.find("distributions")) { + for (JSONNode const &dist : distributions->children()) { + std::string const name = RooJSONFactoryWSTool::name(dist); + if (typeOf(dist) != "combined_dist" || inLikelihoods.count(name)) { + continue; + } + PartVariates parts; + for (std::string const &component : combinedComponents(dist, "dist")) { + parts.emplace_back(component, variatesOf(rootnode, component)); + } + append(sharedVariates(parts, "components", "the combined_dist '" + name + "'")); + } + } + if (JSONNode const *data = rootnode.find("data")) { + for (JSONNode const &dataset : data->children()) { + std::string const name = RooJSONFactoryWSTool::name(dataset); + if (typeOf(dataset) != "combined" || inLikelihoods.count(name)) { + continue; + } + PartVariates parts; + for (std::string const &component : combinedComponents(dataset, "dataset")) { + std::set axes; + if (JSONNode const *node = findInSection(rootnode, "data", component)) { + if (JSONNode const *axesNode = node->find("axes")) { + for (JSONNode const &axis : axesNode->children()) { + if (JSONNode const *axisName = axis.find("name")) { + addNames(*axisName, axes); + } + } + } + } + parts.emplace_back(component, axes); + } + append(sharedVariates(parts, "components", "the combined data set '" + name + "'")); + } + } + return messages; +} + +// Write each term of the likelihoods of the exported document `rootnode` once. A RooFit model often has a constraint +// both as a factor of the product of a channel and as a term of the likelihood (e.g. a constraint of a HistFactory +// modifier, or an external constraint of the ModelConfig); RooFit counts it once. In HS3 it would enter twice, so the +// factor is removed from the product. Only products that no other distribution uses are changed (being a component +// of a combined distribution is not a use), and only factors that are terms of every likelihood that has the product +// as a term. +void writeLikelihoodTermsOnce(JSONNode &rootnode) +{ + JSONNode const *likelihoods = rootnode.find("likelihoods"); + JSONNode const *distributions = rootnode.find("distributions"); + if (!likelihoods || !distributions) { + return; + } + // The products that are terms, with the factors that are terms wherever the product is one. + std::map> removable; + for (JSONNode const &likelihood : likelihoods->children()) { + std::vector const terms = likelihoodTerms(rootnode, likelihood); + for (std::string const &term : terms) { + JSONNode const *node = findInSection(rootnode, "distributions", term); + if (!node || typeOf(*node) != "product_dist" || !node->has_child("factors")) { + continue; + } + std::set factorsThatAreTerms; + for (JSONNode const &factor : (*node)["factors"].children()) { + if (factor.val() != term && std::find(terms.begin(), terms.end(), factor.val()) != terms.end()) { + factorsThatAreTerms.insert(factor.val()); + } + } + auto found = removable.find(term); + if (found == removable.end()) { + removable[term] = factorsThatAreTerms; + } else { + std::set both; + std::set_intersection(found->second.begin(), found->second.end(), factorsThatAreTerms.begin(), + factorsThatAreTerms.end(), std::inserter(both, both.begin())); + found->second = both; + } + } + } + // Products that another distribution uses are left alone. + for (JSONNode const &dist : distributions->children()) { + std::vector used; + for (const char *key : {"factors", "summands"}) { + if (JSONNode const *parts = dist.find(key)) { + for (JSONNode const &part : parts->children()) { + if (isName(part)) { + used.push_back(part.val()); + } + } + } + } + if (typeOf(dist) != "combined_dist") { + for (std::string const &component : combinedComponents(dist, "dist")) { + used.push_back(component); + } + } + for (std::string const &name : used) { + removable.erase(name); + } + } + for (auto const &[product, remove] : removable) { + if (remove.empty()) { + continue; + } + auto &node = const_cast(*findInSection(rootnode, "distributions", product)); + std::vector keep; + for (JSONNode const &factor : node["factors"].children()) { + if (remove.find(factor.val()) == remove.end()) { + keep.push_back(factor.val()); + } + } + if (keep.empty()) { + continue; + } + for (std::string const &factor : remove) { + oocoutI(nullptr, IO) << "RooFitHS3: '" << factor << "' is a factor of '" << product + << "' and a term of its likelihood; it is written once, as a term of the likelihood" + << std::endl; + } + node["factors"].clear(); + node["factors"].fill_seq(keep); + } +} +// --------------------------------------------------------------------------------------------------------------- + // Export the string attributes and tags of `arg` into the ROOT-internal attributes. void exportAttributes(const RooAbsArg *arg, std::map &attributes) { @@ -1213,34 +1567,6 @@ void combineDatasets(HS3Document const &doc, std::vector -std::vector sharedVariableNames(DataSets const &datasets, GetData getData) -{ - std::map counts; - for (auto const &item : datasets) { - for (RooAbsArg const *var : *getData(item)->get()) { - ++counts[var->GetName()]; - } - } - std::vector shared; - for (auto const &[name, count] : counts) { - if (count > 1) { - shared.push_back(name); - } - } - return shared; -} - -std::string joinQuoted(std::vector const &names) -{ - std::string out; - for (std::string const &name : names) { - out += (out.empty() ? "'" : ", '") + name + "'"; - } - return out; -} - // Build the combined data sets of the document: a RooDataSet with one category column per entry of "columns" and the // variables of all components, filled row by row from the components, which are then no longer imported on their own. // The columns are categories declared by categorical axes of the domains. A key without a sub-data set is a state @@ -1289,15 +1615,6 @@ void assembleCombinedData(HS3Document const &doc, RooWorkspace const &workspace, } components.push_back({&c.key, std::move(*found)}); } - // HS3 requires distinct axis names, since the components of a combined distribution share no variate. RooFit - // can still read such a document. - std::vector shared = sharedVariableNames(components, [](auto const &c) { return c.data.get(); }); - if (!shared.empty()) { - RooJSONFactoryWSTool::warning("RooFitHS3: the sub-data sets of " + context + " share the axes " + - joinQuoted(shared) + - ". HS3 requires distinct axis names, because the components of a combined " - "distribution share no variate. RooFit reads the document anyway."); - } // The variables: the columns first, then those of the components. A weight variable if any component is // weighted, with errors if any component is binned, like RooDataSet does when importing a map of data sets. @@ -2299,18 +2616,6 @@ RooJSONFactoryWSTool::CombinedData RooJSONFactoryWSTool::exportCombinedData(RooA combined.components.push_back({key, {subName}}); subDataSets.push_back(found->get()); } - // In HS3, the components of a combined distribution share no variate: a quantity measured in several components is a - // separate variate in each of them, e.g. "mass_SR" and "mass_CR". The sub-data sets, which are split by the - // components of the RooSimultaneous, must therefore not share observables. - if (simPdf) { - std::vector shared = sharedVariableNames(subDataSets, [](RooAbsData const *d) { return d; }); - if (!shared.empty()) { - error("the data set '" + info.name + "' is split by the components of '" + simPdf->GetName() + - "', which share the observables " + joinQuoted(shared) + - ". HS3 requires each component of a combined distribution to have its own variates: give the " - "observable a separate name in each component (e.g. 'mass_SR' and 'mass_CR' instead of 'mass')."); - } - } _export->doc.data.push_back({info.name, std::move(combined)}); for (RooAbsData *subData : subDataSets) { exportData(*subData); @@ -2805,6 +3110,30 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) } HS3::report(diagnostics); HS3::writeDocument(n, doc); + + // Each term of a likelihood once, and distinct variates (see sharedVariates()). Otherwise, the document is only + // written if explicitly allowed, e.g. to rename the variates in the JSON by hand. This is checked on the written + // document, which has the generic distributions. + writeLikelihoodTermsOnce(n); + if (std::vector const shared = sharedVariatesOfDocument(n); !shared.empty()) { + if (!config().allowExportSharedVariates) { + std::string text; + for (std::string const &message : shared) { + text += (text.empty() ? "" : "; ") + message; + } + error("cannot export: " + text + + ". HS3 requires the terms of a likelihood and the components of a combined distribution to have " + "distinct variates: give the variates their own names (e.g. 'mass_SR' and 'mass_CR' instead of " + "'mass'; RooJSONFactoryWSTool::config().renameCombinedObservables does this for the observables of " + "combined data sets), or set RooJSONFactoryWSTool::config().allowExportSharedVariates = true to " + "write the document anyway, with a warning, and rename them in the JSON."); + } + for (std::string const &message : shared) { + warning("RooFitHS3: " + message + + ". HS3 requires distinct variates: the document is not valid HS3, and can't be read back before the " + "variates are renamed."); + } + } } /** @@ -3026,6 +3355,19 @@ void RooJSONFactoryWSTool::importAllNodes(JSONNode &n) // Now, read in analyses and likelihoods if there are any + // Distinct variates (see sharedVariates()). Documents of versions before 0.3, which RooFit wrote with shared + // variates itself, are read with a warning. + for (std::string const &message : sharedVariatesOfDocument(n)) { + if (!isHS3VersionBefore(_hs3VersionInput, 0, 3)) { + error("invalid HS3: " + message + + ". The terms of a likelihood and the components of a combined distribution or data set must have " + "distinct variates."); + } + warning("RooFitHS3: " + message + + ". HS3 0.3 requires distinct variates; the document is read anyway, since it declares an older " + "version."); + } + for (auto const &analysis : doc.analyses) { importAnalysis(doc, analysis, _workspace, datasets); } diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index ed899ff82d49a..b49109929a276 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -1421,7 +1421,9 @@ TEST(RooFitHS3, SimultaneousGaussians) RooGaussian g1("g1", "g1", x, mean, sigma); RooGaussian g2("g2", "g2", x, mean, 0.3); RooProdPdf model("model", "model", RooArgList{g1, g2}); - RooGaussian model_ctl("model_ctl", "model_ctl", x, mean, sigma); + // The components have their own observables: HS3 requires distinct variates of the components. + RooRealVar xCtl("x_ctl", "x_ctl", -8, 8); + RooGaussian model_ctl("model_ctl", "model_ctl", xCtl, mean, sigma); RooCategory sample("sample", "sample", {{"physics", 0}, {"control", 1}}); RooSimultaneous simPdf("simPdf", "simultaneous pdf", sample); simPdf.addPdf(model, "physics"); @@ -5529,7 +5531,8 @@ TEST(RooFitHS3, CombinedDist) RooWorkspace ws; ws.factory("Gaussian::model(x[-8, 8], mean[0, -8, 8], sigma[0.3, 0.1, 10])"); - ws.factory("Gaussian::model_ctl(x, mean, 1.0)"); + // The components have their own observables: HS3 requires distinct variates of the components. + ws.factory("Gaussian::model_ctl(x_ctl[-8, 8], mean, 1.0)"); // The states are defined in this order, with non-contiguous indices. ws.factory("SIMUL::simPdf(sample[physics=3, control=7], physics=model, control=model_ctl)"); @@ -5577,10 +5580,11 @@ TEST(RooFitHS3, CombinedDist) ASSERT_TRUE(RooJSONFactoryWSTool{wsLegacy}.importJSONfromString(R"({ "metadata": {"hs3_version": "0.2"}, "domains": [{"name": "default_domain", "type": "product_domain", - "axes": [{"name": "x", "min": -8.0, "max": 8.0}, {"name": "mean", "min": -8.0, "max": 8.0}]}], + "axes": [{"name": "x", "min": -8.0, "max": 8.0}, {"name": "x_ctl", "min": -8.0, "max": 8.0}, + {"name": "mean", "min": -8.0, "max": 8.0}]}], "distributions": [ {"name": "model", "type": "gaussian_dist", "x": "x", "mean": "mean", "sigma": 0.3}, - {"name": "model_ctl", "type": "gaussian_dist", "x": "x", "mean": "mean", "sigma": 1.0} + {"name": "model_ctl", "type": "gaussian_dist", "x": "x_ctl", "mean": "mean", "sigma": 1.0} ], "misc": {"ROOT_internal": {"combined_distributions": {"simPdf": { "index_cat": "sample", "labels": ["physics", "control"], "indices": [3, 7], @@ -5601,11 +5605,17 @@ TEST(RooFitHS3, CombinedDistSeveralColumns) { RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); - RooRealVar x{"x", "x", 0.0, 10.0}; - x.setBins(5); - RooGaussian sr{"sr", "sr", x, 3.0, 1.0}; - RooGaussian cr{"cr", "cr", x, 6.0, 1.0}; - RooGaussian srGlobal{"sr_global", "sr_global", x, 5.0, 2.0}; + // The components have their own observables: HS3 requires distinct variates of the components. + RooRealVar xSR{"x_sr", "x_sr", 0.0, 10.0}; + RooRealVar xCR{"x_cr", "x_cr", 0.0, 10.0}; + RooRealVar xGlobal{"x_sr_global", "x_sr_global", 0.0, 10.0}; + std::map observableOf{{"sr", &xSR}, {"cr", &xCR}, {"sr_global", &xGlobal}}; + for (auto &item : observableOf) { + item.second->setBins(5); + } + RooGaussian sr{"sr", "sr", xSR, 3.0, 1.0}; + RooGaussian cr{"cr", "cr", xCR, 6.0, 1.0}; + RooGaussian srGlobal{"sr_global", "sr_global", xGlobal, 5.0, 2.0}; // The super category is built over (region, kind); the columns are written sorted by name: (kind, region). RooCategory region{"region", "region", {{"SR", 0}, {"CR", 1}}}; RooCategory kind{"kind", "kind", {{"physics", 0}, {"global", 1}}}; @@ -5639,17 +5649,18 @@ TEST(RooFitHS3, CombinedDistSeveralColumns) ASSERT_NE(simPdf2, nullptr); ASSERT_NE(dynamic_cast(&simPdf2->indexCat()), nullptr); EXPECT_EQ(RooJSONFactoryWSTool{ws2}.exportJSONtoString(), json); - RooArgSet nset{x}; - RooArgSet nset2{*ws2.var("x")}; + RooArgSet nset{xSR, xCR, xGlobal}; + RooArgSet nset2{*ws2.var("x_sr"), *ws2.var("x_cr"), *ws2.var("x_sr_global")}; for (auto const &item : expectedKeys) { kind.setLabel(item.second[0]); region.setLabel(item.second[1]); ws2.cat("kind")->setLabel(item.second[0]); ws2.cat("region")->setLabel(item.second[1]); EXPECT_STREQ(simPdf2->getPdf(simPdf2->indexCat().getCurrentLabel())->GetName(), item.first.c_str()); + RooRealVar &x = *observableOf[item.first]; for (int i = 0; i < x.numBins(); ++i) { x.setBin(i); - ws2.var("x")->setBin(i); + ws2.var(x.GetName())->setBin(i); EXPECT_DOUBLE_EQ(simPdf2->getVal(nset2), simPdf.getVal(nset)) << item.first << " bin " << i; } } @@ -5664,11 +5675,11 @@ TEST(RooFitHS3, CombinedDistImportErrors) return R"({ "metadata": {"hs3_version": "0.3"}, "domains": [{"name": "default_domain", "type": "product_domain", - "axes": [{"name": "x", "min": 0.0, "max": 10.0}, )" + + "axes": [{"name": "x", "min": 0.0, "max": 10.0}, {"name": "x_b", "min": 0.0, "max": 10.0}, )" + axes + R"(]}], "distributions": [ {"name": "ga", "type": "gaussian_dist", "x": "x", "mean": 3.0, "sigma": 1.0}, - {"name": "gb", "type": "gaussian_dist", "x": "x", "mean": 6.0, "sigma": 1.0}, + {"name": "gb", "type": "gaussian_dist", "x": "x_b", "mean": 6.0, "sigma": 1.0}, {"name": "simPdf", "type": "combined_dist", "columns": )" + columns + R"(, "components": )" + components + R"(} ] @@ -5940,10 +5951,11 @@ TEST(RooFitHS3, CombinedDataImportErrors) return R"({ "metadata": {"hs3_version": "0.3"}, "domains": [{"name": "default_domain", "type": "product_domain", - "axes": [{"name": "x", "min": 0.0, "max": 10.0}, {"name": "sample", "categories": ["a", "b"]}]}], + "axes": [{"name": "x", "min": 0.0, "max": 10.0}, {"name": "x_b", "min": 0.0, "max": 10.0}, + {"name": "sample", "categories": ["a", "b"]}]}], "data": [ {"name": "da", "type": "unbinned", "axes": [{"name": "x", "min": 0.0, "max": 10.0}], "entries": [[1.0]]}, - {"name": "db", "type": "unbinned", "axes": [{"name": "x", "min": 0.0, "max": 10.0}], "entries": [[2.0], [3.0]]}, + {"name": "db", "type": "unbinned", "axes": [{"name": "x_b", "min": 0.0, "max": 10.0}], "entries": [[2.0], [3.0]]}, {"name": "obs", "type": "combined", )" + axes + combined + R"(} ] @@ -6176,8 +6188,8 @@ TEST(RooFitHS3, CombinedObjectsKeepNamesWithoutMisc) // In HS3, the components of a combined distribution share no variate: a quantity measured in several components is a // separate variate in each of them ("mass_SR", "mass_CR"). A RooSimultaneous whose components share an observable can -// therefore not be written with its data set. A document whose sub-data sets share an axis is still read, with a -// warning. +// therefore not be written, unless explicitly allowed: then the document is written with a warning. A document whose +// sub-data sets share an axis is rejected, or read with a warning if it declares a version before 0.3. TEST(RooFitHS3, CombinedComponentsMustNotShareObservables) { RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); @@ -6199,9 +6211,22 @@ TEST(RooFitHS3, CombinedComponentsMustNotShareObservables) const std::string errors = captureMessages(RooFit::ERROR, RooFit::IO, [&] { EXPECT_THROW(RooJSONFactoryWSTool{ws}.exportJSONtoString(), std::runtime_error); }); - EXPECT_NE(errors.find("which share the observables 'mass'"), std::string::npos) << errors; + EXPECT_NE(errors.find("the variate 'mass' is shared by the components 'cr' and 'sr' of the combined_dist 'simPdf'"), + std::string::npos) + << errors; EXPECT_NE(errors.find("'mass_SR' and 'mass_CR'"), std::string::npos) << errors; + // Written anyway if allowed, with a warning; the document is rejected on import. + RooJSONFactoryWSTool::config().allowExportSharedVariates = true; + std::string invalid; + const std::string exportWarnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, + [&] { invalid = RooJSONFactoryWSTool{ws}.exportJSONtoString(); }); + RooJSONFactoryWSTool::config().allowExportSharedVariates = false; + EXPECT_NE(exportWarnings.find("the variate 'mass' is shared by the components 'cr' and 'sr'"), std::string::npos) + << exportWarnings; + EXPECT_NE(exportWarnings.find("not valid HS3"), std::string::npos) << exportWarnings; + expectImportThrowsWithError(invalid, "the variate 'mass' is shared by the components 'cr' and 'sr'"); + const std::string json = R"({ "metadata": {"hs3_version": "0.3"}, "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ @@ -6213,11 +6238,16 @@ TEST(RooFitHS3, CombinedComponentsMustNotShareObservables) "components": [{"key": ["SR"], "dataset": "obs_SR"}, {"key": ["CR"], "dataset": "obs_CR"}]} ] })"; + const std::string sharedAxis = + "the variate 'mass' is shared by the components 'obs_SR' and 'obs_CR' of the combined data set 'obs'"; + expectImportThrowsWithError(json, sharedAxis); + std::string older = json; + older.replace(older.find("\"0.3\""), 5, "\"0.2\""); RooWorkspace imported; const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { - EXPECT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + EXPECT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(older)); }); - EXPECT_NE(warnings.find("share the axes 'mass'"), std::string::npos) << warnings; + EXPECT_NE(warnings.find(sharedAxis), std::string::npos) << warnings; RooAbsData *data = imported.data("obs"); ASSERT_NE(data, nullptr); EXPECT_EQ(data->numEntries(), 2); @@ -6314,3 +6344,103 @@ TEST(RooFitHS3, RenameCombinedObservables) } EXPECT_EQ(axes, (std::set{"obs_SR_x", "obs_SR_y"})) << jsonXY; } + +// HS3 requires the terms of a likelihood to have distinct variates, like the components of a combined distribution. +// Two channels with a common observable can't be exported, unless explicitly allowed: then the document is written +// with a warning, and it is rejected on import. +TEST(RooFitHS3, SharedVariatesInLikelihood) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::pdf_a(x[0, -5, 5], mu[0, -5, 5], sigma[1, 0.1, 5])"); + ws.factory("Gaussian::pdf_b(x, mu, sigma)"); + ws.factory("SIMUL::sim_pdf(channel[a=0, b=1], a=pdf_a, b=pdf_b)"); + RooArgSet observables{*ws.var("x"), *ws.cat("channel")}; + RooDataSet data{"data", "data", observables}; + for (int index : {0, 1}) { + ws.cat("channel")->setIndex(index); + data.add(observables); + } + ws.import(data, RooFit::Silence()); + RooFit::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("sim_pdf"); + mc.SetParametersOfInterest("mu"); + mc.SetObservables("x,channel"); + ws.import(mc); + + const std::string message = "the variate 'x' is shared by the terms 'pdf_a' and 'pdf_b' of the likelihood"; + expectExportThrowsWithError(ws, message); + + RooJSONFactoryWSTool::config().allowExportSharedVariates = true; + std::string exported; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, + [&] { exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); }); + RooJSONFactoryWSTool::config().allowExportSharedVariates = false; + EXPECT_NE(warnings.find(message), std::string::npos) << warnings; + EXPECT_NE(warnings.find("not valid HS3"), std::string::npos) << warnings; + EXPECT_FALSE(exported.empty()); + + expectImportThrowsWithError(exported, message); +} + +// A constraint that is both a factor of the model and an external constraint of the ModelConfig is one term of the +// likelihood in RooFit. It is written once, as a term of the likelihood: the product keeps its other factors. The +// document has distinct variates, reads back with the same NLL, and is stable. +TEST(RooFitHS3, ConstraintInModelAndExternal) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::shape(x[0, -5, 5], theta[0, -5, 5], 1.0)"); + ws.factory("Gaussian::constraint(theta_obs[0, -5, 5], theta, 1.0)"); + ws.factory("PROD::model(shape, constraint)"); + ws.var("theta_obs")->setConstant(true); + RooDataSet data{"data", "data", *ws.var("x")}; + ws.var("x")->setVal(0.7); + data.add(*ws.var("x")); + ws.import(data, RooFit::Silence()); + RooFit::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("model"); + mc.SetParametersOfInterest("theta"); + mc.SetObservables("x"); + mc.SetGlobalObservables("theta_obs"); + mc.SetExternalConstraints(RooArgSet{*ws.pdf("constraint")}); + ws.import(mc); + + auto nllOf = [](RooWorkspace &w) { + auto *modelConfig = dynamic_cast(w.obj("ModelConfig")); + if (!modelConfig) { + return std::numeric_limits::quiet_NaN(); + } + w.var("theta")->setVal(0.4); + RooArgSet external; + if (modelConfig->GetExternalConstraints()) { + external.add(*modelConfig->GetExternalConstraints()); + } + std::unique_ptr nll{ + modelConfig->GetPdf()->createNLL(*w.data("data"), RooFit::ExternalConstraints(external))}; + return nll->getVal(); + }; + const double nll = nllOf(ws); + ASSERT_FALSE(std::isnan(nll)); + + const std::string exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(exported); + const auto *model = RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["distributions"], "model"); + ASSERT_NE(model, nullptr) << exported; + ASSERT_EQ((*model)["factors"].num_children(), 1u) << exported; + EXPECT_EQ((*model)["factors"].child(0).val(), "shape") << exported; + EXPECT_NE(exported.find(R"("aux_distributions":["constraint"])"), std::string::npos) << exported; + + RooWorkspace ws2; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(exported)) << exported; + }); + EXPECT_EQ(warnings.find("is shared by the"), std::string::npos) << warnings; + EXPECT_NEAR(nllOf(ws2), nll, 1e-10 * std::abs(nll)); + const std::string exported2 = RooJSONFactoryWSTool{ws2}.exportJSONtoString(); + RooWorkspace ws3; + ASSERT_TRUE(RooJSONFactoryWSTool{ws3}.importJSONfromString(exported2)); + EXPECT_EQ(RooJSONFactoryWSTool{ws3}.exportJSONtoString(), exported2); +} From 6199273792025a1f16af33a8416302e3e446a308 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Wed, 11 Nov 2026 15:29:44 +0100 Subject: [PATCH 11/18] [RF][HS3] Name the axes of histograms after the observables of the function A RooHistFunc or RooHistPdf is evaluated with its own observables, which are mapped by position onto the variables of its histogram and need not have the same names. The exporters took the names of the axes from the variables of the histogram. With RooJSONFactoryWSTool::config().renameCombinedObservables, where a RooCustomizer replaces the observables of the components of a RooSimultaneous, the "histfactory_dist" of a channel therefore kept the original observable "x" as axis while the data had "obs_SR" (reported in the review of this PR, with a reproducer). The same happened for "step" and "histogram_dist", and for any RooHistFunc constructed with other observables than the variables of its histogram. The axes are now named after the observables onto which the variables of the histogram are mapped (histogramVariablesNamedAfterObservables() in JSONIOUtils). The HistFactory exporter keeps these variables for the channel, so the observables are also left out of the parameters of the channel. RooHistPdf gets getHistObsList(), like RooHistFunc. Test: HistogramAxesNamedAfterObservables (a RooHistFunc with observables of other names, a HistFactory channel and a RooHistPdf on the same observable with renamed observables: axes of the distributions and of the data, likelihood after the import, stable document). Ported onto the typed HS3 document: the observables of the exported channel (ExportedChannel::observables) are the renamed variables. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/JSONFactories_HistFactory.cxx | 20 +-- roofit/hs3/src/JSONFactories_RooFitCore.cxx | 5 +- roofit/hs3/src/JSONIOUtils.cxx | 18 +++ roofit/hs3/src/JSONIOUtils.h | 11 ++ roofit/hs3/test/testRooFitHS3.cxx | 121 +++++++++++++++++++ roofit/roofitcore/inc/RooHistPdf.h | 2 + 6 files changed, 169 insertions(+), 8 deletions(-) diff --git a/roofit/hs3/src/JSONFactories_HistFactory.cxx b/roofit/hs3/src/JSONFactories_HistFactory.cxx index 41f4e9a07c094..e37447c6d071c 100644 --- a/roofit/hs3/src/JSONFactories_HistFactory.cxx +++ b/roofit/hs3/src/JSONFactories_HistFactory.cxx @@ -934,7 +934,8 @@ std::vector contents(RooDataHist const &dataHist) struct ExportedChannel { std::string name; HS3::HistFactoryDist dist; - RooArgSet const *observables = nullptr; + // The variables of the first histogram, named after the observables of the channel. + std::unique_ptr observables; /// The pdfs that the modifiers name as constraints, by name. std::map constraints; std::vector customModifiers; @@ -1066,13 +1067,18 @@ ExportedChannel readChannel(RooJSONFactoryWSTool *tool, const std::string &pdfna normSys.push_back({sysname, {param.GetName()}, hi, lo, constraint(pdf), interpolation}); }; - auto updateObservables = [&](RooDataHist const &dataHist) { + auto updateObservables = [&](RooHistFunc const &histFunc) { + RooDataHist const &dataHist = histFunc.dataHist(); + // The axes are named after the observables of the RooHistFunc, which are not necessarily the variables of + // its histogram (e.g. after a RooCustomizer replaced them). + auto varsPtr = + histogramVariablesNamedAfterObservables(dataHist, histFunc.getHistObsList(), histFunc.variables()); if (channel.observables == nullptr) { - channel.observables = dataHist.get(); + channel.observables = std::move(varsPtr); } else { // The bin contents of all samples are written in the order of the axes of the channel (the last axis // varies fastest), which is the order of the observables of the first histogram. - RooArgSet const &vars = *dataHist.get(); + RooArgSet const &vars = *varsPtr; bool sameOrder = vars.size() == channel.observables->size(); for (std::size_t i = 0; sameOrder && i < vars.size(); ++i) { sameOrder = std::string(vars[i]->GetName()) == (*channel.observables)[i]->GetName(); @@ -1123,7 +1129,7 @@ ExportedChannel readChannel(RooJSONFactoryWSTool *tool, const std::string &pdfna normFactors.push_back({par->GetName(), {par->GetName()}, HS3::Unconstrained{}}); } } else if (auto hf = dynamic_cast(e)) { - updateObservables(hf->dataHist()); + updateObservables(*hf); } else if (ParamHistFunc *phf = dynamic_cast(e); phf && allRooRealVar(phf->paramList())) { phfs.push_back(phf); } else if (auto fip = dynamic_cast(e)) { @@ -1200,7 +1206,7 @@ ExportedChannel readChannel(RooJSONFactoryWSTool *tool, const std::string &pdfna // see if we can get the observables if (pip) { if (auto nh = dynamic_cast(pip->nominalHist())) { - updateObservables(nh->dataHist()); + updateObservables(*nh); } } @@ -1416,7 +1422,7 @@ bool tryExportHistFactory(RooJSONFactoryWSTool *tool, const std::string &pdfname // Export all model parameters RooArgSet parameters; - sumpdf->getParameters(channel.observables, parameters); + sumpdf->getParameters(channel.observables.get(), parameters); for (RooAbsArg *param : parameters) { // This should exclude the global observables. The legacy inverse bin widths are not written at all. if (!startsWith(std::string{param->GetName()}, "nom_") && !channel.legacyBinWidthFactors.count(param)) { diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index 4f7dfe008ac4a..1725355ac39b7 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -1110,7 +1110,10 @@ bool exportHist(RooJSONFactoryWSTool *tool, const RooAbsArg *func, JSONNode &ele const RooArg_t *hf = static_cast(func); elem["type"] << key; RooDataHist const &dh = hf->dataHist(); - tool->exportHisto(*dh.get(), dh.numEntries(), dh.weightArray(), elem["data"].set_map()); + // The axes are named after the observables of the function, which are not necessarily the variables of the + // histogram. + auto vars = histogramVariablesNamedAfterObservables(dh, hf->getHistObsList(), hf->variables()); + tool->exportHisto(*vars, dh.numEntries(), dh.weightArray(), elem["data"].set_map()); return true; } diff --git a/roofit/hs3/src/JSONIOUtils.cxx b/roofit/hs3/src/JSONIOUtils.cxx index 07ca32c23c6e3..260d9355dd31d 100644 --- a/roofit/hs3/src/JSONIOUtils.cxx +++ b/roofit/hs3/src/JSONIOUtils.cxx @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -120,6 +121,23 @@ std::size_t th1Position(std::size_t k, std::vector const &nBins) } // namespace +std::unique_ptr +histogramVariablesNamedAfterObservables(RooDataHist const &dh, RooArgSet const &histObs, RooArgSet const &funcObs) +{ + auto out = std::make_unique(); + for (RooAbsArg *var : *dh.get()) { + std::string name = var->GetName(); + for (std::size_t i = 0; i < histObs.size() && i < funcObs.size(); ++i) { + if (name == histObs[i]->GetName() && dynamic_cast(funcObs[i])) { + name = funcObs[i]->GetName(); + break; + } + } + out->addOwned(std::unique_ptr{static_cast(var->clone(name.c_str()))}); + } + return out; +} + std::vector paramHistFuncParamsInBinOrder(ParamHistFunc const &func) { std::vector out; diff --git a/roofit/hs3/src/JSONIOUtils.h b/roofit/hs3/src/JSONIOUtils.h index cf231c5c1c2c5..a6aa81867c099 100644 --- a/roofit/hs3/src/JSONIOUtils.h +++ b/roofit/hs3/src/JSONIOUtils.h @@ -1,6 +1,7 @@ #ifndef JSONIOUtils_h #define JSONIOUtils_h +#include #include #include #include @@ -10,7 +11,9 @@ class RooAbsCategory; class RooAbsCollection; class RooAbsReal; class RooArgList; +class RooArgSet; class RooCategory; +class RooDataHist; class RooWorkspace; class ParamHistFunc; @@ -25,6 +28,14 @@ inline constexpr const char *fixedValueKey = "value"; bool parseHS3Version(std::string const &version, int &major, int &minor); bool isHS3VersionBefore(std::string const &version, int major, int minor); +// A RooHistFunc or RooHistPdf is evaluated with its own observables (`funcObs`), which are mapped by position onto +// the variables of its histogram (`histObs`) and need not have the same names, e.g. after a RooCustomizer replaced +// them. The variables of the histogram `dh`, in the order of the histogram, each named after the observable that is +// mapped onto it, so that the axes are written with the names of the observables. A variable onto which no variable +// or category is mapped keeps its name. +std::unique_ptr +histogramVariablesNamedAfterObservables(RooDataHist const &dh, RooArgSet const &histObs, RooArgSet const &funcObs); + // ParamHistFunc orders its parameters like the bins of a TH1 (first variable fastest), while HS3 lays out per-bin // arrays with the last axis varying fastest, like RooDataHist. These helpers convert at the HS3 boundary. std::vector paramHistFuncParamsInBinOrder(ParamHistFunc const &func); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index b49109929a276..57d33b5962f7d 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6345,6 +6345,127 @@ TEST(RooFitHS3, RenameCombinedObservables) EXPECT_EQ(axes, (std::set{"obs_SR_x", "obs_SR_y"})) << jsonXY; } +// Histograms are written with axes named after the observables of the RooHistFunc or RooHistPdf, which are mapped +// onto the variables of the histogram and need not have the same names. This matters when the observables are +// renamed per key (reported in the review): the "histfactory_dist" of a channel and a "histogram_dist" have the +// renamed observables as axes, like the data, and the renamed observables are not written as parameters. +TEST(RooFitHS3, HistogramAxesNamedAfterObservables) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + // A RooHistFunc with observables of other names than the variables of its histogram is written with the names of + // its observables. + { + RooRealVar u{"u", "u", 0.0, 2.0}; + u.setBins(2); + RooRealVar v{"v", "v", 0.0, 2.0}; + v.setBins(2); + std::unique_ptr dh = makeDataHist("dh_v", v, {1.0, 3.0}); + RooHistFunc func{"func", "func", RooArgList{u}, RooArgList{v}, *dh}; + RooWorkspace ws; + ws.import(func, RooFit::Silence()); + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const *node = RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["functions"], "func"); + ASSERT_NE(node, nullptr) << json; + ASSERT_EQ((*node)["data"]["axes"].num_children(), 1u) << json; + EXPECT_EQ((*node)["data"]["axes"].child(0)["name"].val(), "u") << json; + } + + struct FlagGuard { + bool old = RooJSONFactoryWSTool::config().renameCombinedObservables; + FlagGuard() { RooJSONFactoryWSTool::config().renameCombinedObservables = true; } + ~FlagGuard() { RooJSONFactoryWSTool::config().renameCombinedObservables = old; } + } flagGuard; + + // A HistFactory channel (SR) and a RooHistPdf (CR) on the same observable. + RooRealVar x{"x", "x", 0.0, 2.0}; + x.setBins(2); + std::unique_ptr template_ = makeDataHist("template", x, {10.0, 20.0}); + RooHistFunc shape{"shape", "shape", RooArgSet{x}, *template_}; + RooBinWidthFunction width{"width", "width", shape, true}; + RooProduct sample{"sample", "sample", {shape, width}}; + RooRealVar mu{"mu", "mu", 1.0, 0.1, 3.0}; + RooRealSumPdf model{"model_SR", "model_SR", {sample}, {mu}, true}; + std::unique_ptr crHist = makeDataHist("crHist", x, {5.0, 1.0}); + RooHistPdf cr{"cr", "cr", RooArgSet{x}, *crHist}; + RooCategory channel{"channel", "channel", {{"SR", 0}, {"CR", 1}}}; + RooSimultaneous simPdf{"simPdf", "simPdf", {{"SR", &model}, {"CR", &cr}}, channel}; + // Binned data in the usual HistFactory form, one weighted entry per bin. + RooRealVar weight{"weight", "weight", 1.0}; + RooDataSet obsData{"obsData", "obsData", {x, channel, weight}, RooFit::WeightVar(weight)}; + for (auto const &[label, value, w] : + std::vector>{{"SR", 0.5, 1.0}, {"SR", 1.5, 2.0}, {"CR", 0.2, 1.0}}) { + channel.setLabel(label); + x.setVal(value); + obsData.add({x, channel}, w); + } + + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + ws.import(obsData, RooFit::Silence()); + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const &root = tree->rootnode(); + + auto axisNames = [](RooFit::Detail::JSONNode const &axes) { + std::vector names; + for (auto const &axis : axes.children()) { + names.push_back(axis["name"].val()); + } + return names; + }; + std::map> distAxes; + for (auto const &dist : root["distributions"].children()) { + if (dist["type"].val() == "histfactory_dist") { + distAxes[dist["type"].val()] = axisNames(dist["axes"]); + } else if (dist["type"].val() == "histogram_dist") { + distAxes[dist["type"].val()] = axisNames(dist["data"]["axes"]); + } + } + EXPECT_EQ(distAxes, (std::map>{{"histfactory_dist", {"obs_SR"}}, + {"histogram_dist", {"obs_CR"}}})) + << json; + for (const char *key : {"SR", "CR"}) { + auto const *part = RooJSONFactoryWSTool::findNamedChild(root["data"], std::string("obsData_") + key); + ASSERT_NE(part, nullptr) << json; + EXPECT_EQ(axisNames((*part)["axes"]), std::vector{std::string("obs_") + key}) << json; + } + // The original observable is gone from the document. + for (auto const &domain : root["domains"].children()) { + EXPECT_EQ(axisNames(domain["axes"]), (std::vector{"channel", "mu", "obs_CR", "obs_SR"})) << json; + } + for (auto const &point : root["parameter_points"].children()) { + for (auto const ¶m : point["parameters"].children()) { + EXPECT_NE(param["name"].val(), "x") << json; + } + } + + // Read back, the likelihood changes the same way with the parameter, and the document is stable from the second + // export on (the HistFactory importer adds its attributes and step widths). + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + auto nllDifference = [](RooAbsPdf &pdf, RooAbsData &data, RooRealVar ¶m) { + std::unique_ptr nll{pdf.createNLL(data)}; + const double nominal = nll->getVal(); + const double old = param.getVal(); + param.setVal(1.5); + const double difference = nll->getVal() - nominal; + param.setVal(old); + return difference; + }; + ASSERT_NE(imported.pdf("simPdf"), nullptr); + ASSERT_NE(imported.data("obsData"), nullptr); + ASSERT_NE(imported.var("mu"), nullptr); + const double expected = nllDifference(simPdf, obsData, mu); + EXPECT_NE(expected, 0.0); + EXPECT_NEAR(nllDifference(*imported.pdf("simPdf"), *imported.data("obsData"), *imported.var("mu")), expected, 1e-8); + const std::string json2 = RooJSONFactoryWSTool{imported}.exportJSONtoString(); + RooWorkspace imported2; + ASSERT_TRUE(RooJSONFactoryWSTool{imported2}.importJSONfromString(json2)); + EXPECT_EQ(RooJSONFactoryWSTool{imported2}.exportJSONtoString(), json2); +} + // HS3 requires the terms of a likelihood to have distinct variates, like the components of a combined distribution. // Two channels with a common observable can't be exported, unless explicitly allowed: then the document is written // with a warning, and it is rejected on import. diff --git a/roofit/roofitcore/inc/RooHistPdf.h b/roofit/roofitcore/inc/RooHistPdf.h index 7c1d0f560a54c..2ca27da4e22cf 100644 --- a/roofit/roofitcore/inc/RooHistPdf.h +++ b/roofit/roofitcore/inc/RooHistPdf.h @@ -94,6 +94,8 @@ class RooHistPdf : public RooAbsPdf { void doEval(RooFit::EvalContext &) const override; + RooArgSet const &getHistObsList() const { return _histObsList; } + RooArgSet const &variables() const { return _pdfObsList; } protected: From 9624a6c2e7029648725b5e4f49c79c70ca7f9873 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Wed, 11 Nov 2026 16:01:08 +0100 Subject: [PATCH 12/18] [RF][HS3] Write weighted data sets as binned data only with one entry per bin A one-dimensional weighted RooDataSet is written as binned data if its entries are the bins in their order, the form in which combined data sets of HistFactory models are often stored. The check stopped at the first entry that was not in the next bin and only compared the number of entries before it to the number of bins: entries in the bins 0, 1, 1 counted as binned data, and the export wrote one content per entry, reading past the end of the bin contents. This happened on the second export of the reproducer in the review of this PR, where a HistFactory channel is paired with unbinned data. The data set is now binned data only if it has exactly one entry per bin. Test: WeightedDataSetAsBinnedData. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/RooJSONFactoryWSTool.cxx | 6 ++-- roofit/hs3/test/testRooFitHS3.cxx | 37 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index abe3d4be43842..d0fed87decd77 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -2699,10 +2699,12 @@ void RooJSONFactoryWSTool::exportData(RooAbsData const &data) break; contents.push_back(data.weight()); } - if (i == x.getBins()) + // Binned data has exactly one entry per bin, in the order of the bins. Further entries, e.g. a second one in + // the last bin, make it unbinned data. + if (i == x.getBins() && data.numEntries() == x.getBins()) isBinnedData = true; if (isBinnedData) { - return exportBinned(data.numEntries(), contents.data()); + return exportBinned(contents.size(), contents.data()); } } diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 57d33b5962f7d..71a5d4e600a4e 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6466,6 +6466,43 @@ TEST(RooFitHS3, HistogramAxesNamedAfterObservables) EXPECT_EQ(RooJSONFactoryWSTool{imported2}.exportJSONtoString(), json2); } +// A weighted RooDataSet with one entry per bin, in the order of the bins, is written as binned data. With further +// entries it is unbinned data: a second entry in the last bin was mistaken for binned data, and a value after the +// last bin content was written (reported in the review). +TEST(RooFitHS3, WeightedDataSetAsBinnedData) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + auto exportOne = [](std::vector> const &entries) { + RooRealVar x{"x", "x", 0.0, 2.0}; + x.setBins(2); + RooRealVar weight{"weight", "weight", 1.0}; + RooDataSet data{"data", "data", {x, weight}, RooFit::WeightVar(weight)}; + for (auto const &[value, w] : entries) { + x.setVal(value); + data.add(x, w); + } + RooWorkspace ws; + ws.import(data, RooFit::Silence()); + return RooJSONFactoryWSTool{ws}.exportJSONtoString(); + }; + auto dataNode = [](std::string const &json) { + auto tree = RooFit::Detail::JSONTree::create(json); + std::stringstream ss; + tree->rootnode()["data"].child(0).writeJSON(ss); + return ss.str(); + }; + + const std::string binned = exportOne({{0.5, 1.0}, {1.5, 2.0}}); + EXPECT_NE(dataNode(binned).find(R"("type":"binned")"), std::string::npos) << binned; + EXPECT_NE(dataNode(binned).find(R"("contents":[1,2])"), std::string::npos) << binned; + + // Entries in the bins 0, 1, 1. + const std::string unbinned = exportOne({{0.5, 1.0}, {1.5, 1.0}, {1.7, 1.0}}); + EXPECT_NE(dataNode(unbinned).find(R"("type":"unbinned")"), std::string::npos) << unbinned; + EXPECT_NE(dataNode(unbinned).find(R"("entries":[[0.5],[1.5],[1.7]])"), std::string::npos) << unbinned; +} + // HS3 requires the terms of a likelihood to have distinct variates, like the components of a combined distribution. // Two channels with a common observable can't be exported, unless explicitly allowed: then the document is written // with a warning, and it is rejected on import. From ce1879998e245ef17679b4185d2e694fafd5dd47 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Wed, 11 Nov 2026 16:28:24 +0100 Subject: [PATCH 13/18] [RF][HS3] Declare columns that no distribution uses as bookkeeping columns RooAbsData::split() keeps a column of a data set that no channel of the RooSimultaneous uses (e.g. an event number) in every sub-data set, so all components of the combined data set have it as an axis. HS3 requires distinct axes, and such a data set could not be exported unless a likelihood listed it, since only then were the variates of the distributions checked instead of the axes of the data. Such a column has no statistical meaning. On export, an axis that several components of a combined data set share is declared as a bookkeeping column in misc.ROOT_internal.bookkeeping_columns ({"": ["eventNumber"]}) if its variable is used by nothing else in the workspace: it has no clients and is in no set of a ModelConfig. Bookkeeping columns are exempt from the requirement of distinct axes; all other shared axes are an error. The axes of combined data sets are now checked whether or not a likelihood lists the data set. On import, a declared column is accepted with a warning, and it is not added to the observables of the ModelConfig, so the document is stable. A declared column that is a variate of a distribution is no bookkeeping column. Test: CombinedDataColumnWithoutDistribution (with and without likelihood: the declaration, import with warning, stable document, the same document without declaration rejected; a column in the observables of the ModelConfig and a declared column that is a variate are errors). Ported onto the typed HS3 document: misc.ROOT_internal.bookkeeping_columns is a field of RooFitInternal, which the parser reads and the writer writes. The check works on the JSON tree like the one of distinct variates, and the importer of an analysis gets the bookkeeping columns from the import of the document. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/HS3Document.h | 3 + roofit/hs3/src/HS3Parser.cxx | 1 + roofit/hs3/src/HS3Writer.cxx | 6 + roofit/hs3/src/RooJSONFactoryWSTool.cxx | 150 +++++++++++++++++++++--- roofit/hs3/test/testRooFitHS3.cxx | 110 +++++++++++++++++ 5 files changed, 254 insertions(+), 16 deletions(-) diff --git a/roofit/hs3/src/HS3Document.h b/roofit/hs3/src/HS3Document.h index 5709a3f357b51..fce5b36ebe835 100644 --- a/roofit/hs3/src/HS3Document.h +++ b/roofit/hs3/src/HS3Document.h @@ -485,6 +485,9 @@ struct RooFitInternal { /// The constraint terms of HistFactory distributions that no modifier names, by distribution. They are /// also listed in the "aux_distributions" of the likelihoods, which is what other implementations use. std::map> channelConstraints; + /// Axes that the components of a combined data set share, but that are no variate of any distribution, e.g. an + /// event number, by combined data set. They have no statistical meaning, and may be shared. + std::map> bookkeepingColumns; }; struct Diagnostic { diff --git a/roofit/hs3/src/HS3Parser.cxx b/roofit/hs3/src/HS3Parser.cxx index 78b622b2da90a..520e928dce1ec 100644 --- a/roofit/hs3/src/HS3Parser.cxx +++ b/roofit/hs3/src/HS3Parser.cxx @@ -1226,6 +1226,7 @@ void readMisc(Cursor const &root, RooFitInternal &out) readMap(ar, "combined_datasets", out.combinedDatasets); readMap(ar, "ModelConfigs", out.modelConfigs); readMap(ar, "histfactory_constraints", out.channelConstraints); + readMap(ar, "bookkeeping_columns", out.bookkeepingColumns); } void readMetadata(Cursor const &root, Metadata &out, ParseOptions const &options) diff --git a/roofit/hs3/src/HS3Writer.cxx b/roofit/hs3/src/HS3Writer.cxx index 58d791ce43b62..e2ef6649dc759 100644 --- a/roofit/hs3/src/HS3Writer.cxx +++ b/roofit/hs3/src/HS3Writer.cxx @@ -386,6 +386,12 @@ void writeRooFitInternal(JSONNode &root, RooFitInternal const &internal) writeRefs(constraints[item.first], item.second); } } + if (!internal.bookkeepingColumns.empty()) { + JSONNode &columns = rootInternal()["bookkeeping_columns"].set_map(); + for (auto const &item : internal.bookkeepingColumns) { + writeStrings(columns[item.first], item.second); + } + } if (!internal.stepWidths.empty()) { JSONNode &stepWidths = root.get("misc", "minimization")["parameter_stepwidths"].set_seq(); for (auto const &stepWidth : internal.stepWidths) { diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index d0fed87decd77..6c9ecc512d135 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -818,8 +818,57 @@ std::vector sharedVariates(PartVariates const &parts, std::string c return messages; } -// All shared variates of the document: of the terms of each likelihood, and of the components of the combined -// distributions and combined data sets that no likelihood lists (the others are checked through the likelihood). +// The axes of the components of a combined data set, by component. +PartVariates combinedDataAxes(JSONNode const &rootnode, JSONNode const &dataset) +{ + PartVariates parts; + for (std::string const &component : combinedComponents(dataset, "dataset")) { + std::set axes; + if (JSONNode const *node = findInSection(rootnode, "data", component)) { + if (JSONNode const *axesNode = node->find("axes")) { + for (JSONNode const &axis : axesNode->children()) { + if (JSONNode const *axisName = axis.find("name")) { + addNames(*axisName, axes); + } + } + } + } + parts.emplace_back(component, axes); + } + return parts; +} + +// Bookkeeping columns (a RooFit extension, in misc.ROOT_internal.bookkeeping_columns as {"": +// [, ...]}): axes that several components of a combined data set share, but that are no variate of any +// distribution, e.g. an event number. They have no statistical meaning, and are exempt from the requirement of +// distinct axes. A declared column that is a variate of a distribution is not a bookkeeping column. +std::map> bookkeepingColumnsOfDocument(JSONNode const &rootnode) +{ + std::map> out; + JSONNode const *declared = findRooFitInternal(rootnode, "bookkeeping_columns"); + if (!declared || !declared->is_map()) { + return out; + } + std::set variates; + if (JSONNode const *distributions = rootnode.find("distributions")) { + for (JSONNode const &dist : distributions->children()) { + std::set const v = variatesOf(rootnode, RooJSONFactoryWSTool::name(dist)); + variates.insert(v.begin(), v.end()); + } + } + for (JSONNode const &entry : declared->children()) { + for (JSONNode const &column : entry.children()) { + if (!variates.count(column.val())) { + out[entry.key()].insert(column.val()); + } + } + } + return out; +} + +// All shared variates of the document: of the terms of each likelihood, of the components of the combined +// distributions that no likelihood lists (the others are checked through the likelihood), and the shared axes of the +// components of all combined data sets, except for bookkeeping columns. std::vector sharedVariatesOfDocument(JSONNode const &rootnode) { std::vector messages; @@ -856,25 +905,22 @@ std::vector sharedVariatesOfDocument(JSONNode const &rootnode) append(sharedVariates(parts, "components", "the combined_dist '" + name + "'")); } } + // The axes of the components of every combined data set, also of those that a likelihood lists, except for the + // bookkeeping columns. + std::map> const bookkeeping = bookkeepingColumnsOfDocument(rootnode); if (JSONNode const *data = rootnode.find("data")) { for (JSONNode const &dataset : data->children()) { std::string const name = RooJSONFactoryWSTool::name(dataset); - if (typeOf(dataset) != "combined" || inLikelihoods.count(name)) { + if (typeOf(dataset) != "combined") { continue; } - PartVariates parts; - for (std::string const &component : combinedComponents(dataset, "dataset")) { - std::set axes; - if (JSONNode const *node = findInSection(rootnode, "data", component)) { - if (JSONNode const *axesNode = node->find("axes")) { - for (JSONNode const &axis : axesNode->children()) { - if (JSONNode const *axisName = axis.find("name")) { - addNames(*axisName, axes); - } - } + PartVariates parts = combinedDataAxes(rootnode, dataset); + if (auto found = bookkeeping.find(name); found != bookkeeping.end()) { + for (auto &part : parts) { + for (std::string const &column : found->second) { + part.second.erase(column); } } - parts.emplace_back(component, axes); } append(sharedVariates(parts, "components", "the combined data set '" + name + "'")); } @@ -882,6 +928,61 @@ std::vector sharedVariatesOfDocument(JSONNode const &rootnode) return messages; } +// Declare the bookkeeping columns of the exported document `rootnode` (see bookkeepingColumnsOfDocument()): the axes +// that several components of a combined data set share, and whose variable in the workspace `ws` is used by nothing +// else, i.e. has no clients and is in no set of a ModelConfig. RooAbsData::split() keeps such a column (e.g. an event +// number) in every sub-data set. +void declareBookkeepingColumns(JSONNode &rootnode, RooWorkspace const &ws) +{ + JSONNode const *data = rootnode.find("data"); + if (!data) { + return; + } + std::vector modelConfigSets; + for (TObject *obj : ws.allGenericObjects()) { + if (auto *mc = dynamic_cast(obj)) { + for (RooArgSet const *set : {mc->GetObservables(), mc->GetParametersOfInterest(), mc->GetNuisanceParameters(), + mc->GetGlobalObservables(), mc->GetConditionalObservables()}) { + if (set) { + modelConfigSets.push_back(set); + } + } + } + } + auto usedByNothing = [&](std::string const &name) { + RooAbsArg const *arg = ws.arg(name.c_str()); + if (!arg || !arg->clients().empty()) { + return false; + } + return std::none_of(modelConfigSets.begin(), modelConfigSets.end(), + [&](RooArgSet const *set) { return set->find(name.c_str()) != nullptr; }); + }; + std::map> columns; + for (JSONNode const &dataset : data->children()) { + if (typeOf(dataset) != "combined") { + continue; + } + std::map counts; + for (auto const &[component, axes] : combinedDataAxes(rootnode, dataset)) { + for (std::string const &axis : axes) { + ++counts[axis]; + } + } + for (auto const &[axis, count] : counts) { + if (count > 1 && usedByNothing(axis)) { + columns[RooJSONFactoryWSTool::name(dataset)].insert(axis); + } + } + } + if (columns.empty()) { + return; + } + JSONNode &node = rootnode["misc"]["ROOT_internal"]["bookkeeping_columns"].set_map(); + for (auto const &[dataset, names] : columns) { + node[dataset].fill_seq(names); + } +} + // Write each term of the likelihoods of the exported document `rootnode` once. A RooFit model often has a constraint // both as a factor of the product of a channel and as a term of the likelihood (e.g. a constraint of a HistFactory // modifier, or an external constraint of the ModelConfig); RooFit counts it once. In HS3 it would enter twice, so the @@ -1309,7 +1410,8 @@ void checkCombinedPairing(RooSimultaneous const &simPdf, std::string const &data // Import an analysis (likelihood + domains) as one or more ModelConfig objects into the workspace. void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWorkspace &workspace, - const std::vector> &datasets) + const std::vector> &datasets, + std::set const &bookkeepingColumns) { // if this is a toplevel pdf, also create a modelConfig for it std::string const &analysisName = analysis.name; @@ -1383,6 +1485,12 @@ void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWo if (!found) throw std::runtime_error("dataset '" + dataRef->name + "' cannot be found!"); } + // Bookkeeping columns of the data sets are no observables of the model. + for (std::string const &column : bookkeepingColumns) { + if (RooAbsArg *arg = observables.find(column.c_str())) { + observables.remove(*arg); + } + } // A combined distribution paired with a combined data set must have a sub-data set for every component. for (auto const &term : likelihood.terms) { @@ -3117,6 +3225,7 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n) // written if explicitly allowed, e.g. to rename the variates in the JSON by hand. This is checked on the written // document, which has the generic distributions. writeLikelihoodTermsOnce(n); + declareBookkeepingColumns(n, _workspace); if (std::vector const shared = sharedVariatesOfDocument(n); !shared.empty()) { if (!config().allowExportSharedVariates) { std::string text; @@ -3359,6 +3468,15 @@ void RooJSONFactoryWSTool::importAllNodes(JSONNode &n) // Distinct variates (see sharedVariates()). Documents of versions before 0.3, which RooFit wrote with shared // variates itself, are read with a warning. + std::set bookkeepingColumns; + for (auto const &[dataset, columns] : bookkeepingColumnsOfDocument(n)) { + for (std::string const &column : columns) { + warning("RooFitHS3: the axis '" + column + "' of the components of the combined data set '" + dataset + + "' is declared as a bookkeeping column without statistical meaning (misc.ROOT_internal." + "bookkeeping_columns), so it may be shared by the components, although HS3 requires distinct axes."); + bookkeepingColumns.insert(column); + } + } for (std::string const &message : sharedVariatesOfDocument(n)) { if (!isHS3VersionBefore(_hs3VersionInput, 0, 3)) { error("invalid HS3: " + message + @@ -3371,7 +3489,7 @@ void RooJSONFactoryWSTool::importAllNodes(JSONNode &n) } for (auto const &analysis : doc.analyses) { - importAnalysis(doc, analysis, _workspace, datasets); + importAnalysis(doc, analysis, _workspace, datasets, bookkeepingColumns); } combineDatasets(doc, datasets); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 71a5d4e600a4e..5901705b95d39 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6253,6 +6253,116 @@ TEST(RooFitHS3, CombinedComponentsMustNotShareObservables) EXPECT_EQ(data->numEntries(), 2); } +// A combined data set may have a column that no distribution models, e.g. an event number. RooAbsData::split() keeps +// such a column in every sub-data set, so all components of the combined data set have it as axis, which HS3 doesn't +// allow. If its variable is used by nothing else in the workspace, it is declared as a bookkeeping column in +// misc.ROOT_internal.bookkeeping_columns, whether or not a likelihood lists the data set, and the document is read with +// a warning. A column in a set of the ModelConfig is no bookkeeping column, and neither is a declared column that is a +// variate of a distribution. +TEST(RooFitHS3, CombinedDataColumnWithoutDistribution) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooRealVar xSR{"x_SR", "x_SR", 0.0, 10.0}; + RooRealVar xCR{"x_CR", "x_CR", 0.0, 10.0}; + RooRealVar eventNumber{"eventNumber", "eventNumber", 0.0, 1e6}; + RooGaussian sr{"sr", "sr", xSR, 5.0, 1.0}; + RooGaussian cr{"cr", "cr", xCR, 4.0, 2.0}; + RooCategory channel{"channel", "channel", {{"SR", 0}, {"CR", 1}}}; + RooSimultaneous simPdf{"simPdf", "simPdf", {{"SR", &sr}, {"CR", &cr}}, channel}; + RooDataSet obsData{"obsData", "obsData", {xSR, xCR, eventNumber, channel}}; + int event = 0; + for (auto const &[label, value] : + std::vector>{{"SR", 5.5}, {"CR", 3.0}, {"CR", 6.0}}) { + channel.setLabel(label); + xSR.setVal(value); + xCR.setVal(value); + eventNumber.setVal(event++); + obsData.add({xSR, xCR, eventNumber, channel}); + } + + for (bool withLikelihood : {false, true}) { + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + ws.import(obsData, RooFit::Silence()); + if (withLikelihood) { + RooStats::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("simPdf"); + mc.SetObservables(RooArgSet{xSR, xCR, channel}); + ws.import(mc); + } + std::string json; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_NO_THROW(json = RooJSONFactoryWSTool{ws}.exportJSONtoString()) << withLikelihood; + }); + EXPECT_EQ(warnings.find("is shared by"), std::string::npos) << warnings; + EXPECT_EQ(json.find(R"("likelihoods")") != std::string::npos, withLikelihood) << json; + auto tree = RooFit::Detail::JSONTree::create(json); + for (const char *part : {"obsData_SR", "obsData_CR"}) { + auto const *node = RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["data"], part); + ASSERT_NE(node, nullptr) << json; + EXPECT_NE(RooJSONFactoryWSTool::findNamedChild((*node)["axes"], "eventNumber"), nullptr) << json; + } + + EXPECT_NE(json.find(R"("bookkeeping_columns":{"obsData":["eventNumber"]})"), std::string::npos) << json; + + RooWorkspace imported; + const std::string importWarnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)) << withLikelihood; + }); + EXPECT_NE(importWarnings.find("the axis 'eventNumber' of the components of the combined data set 'obsData' is " + "declared as a bookkeeping column"), + std::string::npos) + << importWarnings; + RooAbsData *data = imported.data("obsData"); + ASSERT_NE(data, nullptr); + EXPECT_EQ(data->numEntries(), 3); + EXPECT_NE(data->get()->find("eventNumber"), nullptr); + // Stable. + std::string json2; + captureMessages(RooFit::WARNING, RooFit::ObjectHandling, + [&] { json2 = RooJSONFactoryWSTool{imported}.exportJSONtoString(); }); + EXPECT_EQ(json2, json); + + // Without the declaration, the document is not valid HS3. + std::string undeclared = json; + undeclared.replace(undeclared.find("bookkeeping_columns"), std::string("bookkeeping_columns").size(), + "other_columns"); + expectImportThrowsWithError(undeclared, "the variate 'eventNumber' is shared by the components"); + } + + // A column in the observables of the ModelConfig is no bookkeeping column. + { + RooWorkspace ws; + ws.import(simPdf, RooFit::Silence()); + ws.import(obsData, RooFit::Silence()); + RooStats::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("simPdf"); + mc.SetObservables(RooArgSet{xSR, xCR, eventNumber, channel}); + ws.import(mc); + const std::string errors = captureMessages(RooFit::ERROR, RooFit::IO, [&] { + EXPECT_THROW(RooJSONFactoryWSTool{ws}.exportJSONtoString(), std::runtime_error); + }); + EXPECT_NE(errors.find("the variate 'eventNumber' is shared by the components"), std::string::npos) << errors; + } + + // A declared column that is a variate of a distribution is no bookkeeping column. + const std::string json = R"({ + "metadata": {"hs3_version": "0.3"}, + "distributions": [{"name": "g", "type": "gaussian_dist", "x": "mass", "mean": 1.0, "sigma": 1.0}], + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "channel", "categories": ["SR", "CR"]}, {"name": "mass", "min": 0.0, "max": 10.0}]}], + "data": [ + {"name": "obs_SR", "type": "unbinned", "axes": [{"name": "mass", "min": 0.0, "max": 10.0}], "entries": [[4.0]]}, + {"name": "obs_CR", "type": "unbinned", "axes": [{"name": "mass", "min": 0.0, "max": 10.0}], "entries": [[5.0]]}, + {"name": "obs", "type": "combined", "columns": ["channel"], + "components": [{"key": ["SR"], "dataset": "obs_SR"}, {"key": ["CR"], "dataset": "obs_CR"}]} + ], + "misc": {"ROOT_internal": {"bookkeeping_columns": {"obs": ["mass"]}}} + })"; + expectImportThrowsWithError(json, "the variate 'mass' is shared by the components 'obs_SR' and 'obs_CR'"); +} + // With RooJSONFactoryWSTool::config().renameCombinedObservables, the observables of combined data sets are renamed // per key on export ("obs_SR", or "obs_SR_x" with several observables), and the components of the matching // RooSimultaneous are written with them; nodes that depend on a renamed observable get one copy per component. This From 8f7aa865d39ed22afcf7c122be735df2e696786f Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Sat, 14 Nov 2026 22:43:32 +0100 Subject: [PATCH 14/18] [RF][HS3] Keep the roles of the parameters of external constraints on import The global observables and nuisance parameters of an imported analysis were determined from the parameters of its pdf only. Parameters that only an external constraint of the ModelConfig depends on, like the global observable and the nuisance parameter of a constraint that is not part of the pdf, lost their roles, which changed the likelihood. The parameters of the external constraints are now taken into account. Reported by Phmonski in https://github.com/root-project/root/pull/24005#issuecomment-6877008344. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/RooJSONFactoryWSTool.cxx | 9 ++++++ roofit/hs3/test/testRooFitHS3.cxx | 37 +++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 6c9ecc512d135..0f42f4d822c42 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -1570,6 +1570,15 @@ void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWo RooArgSet mainPars{pars}; pdf->getAllConstraints(observables, mainPars, /*stripDisconnected*/ true); + // Parameters that only the external constraints depend on are parameters of the likelihood as well, e.g. the + // global observable and the nuisance parameter of a constraint that is not part of the pdf. They are not part of + // the main measurement. + for (RooAbsArg const *extConstraint : extConstraints) { + RooArgSet extPars; + extConstraint->getParameters(&observables, extPars); + pars.add(extPars, /*silent=*/true); + } + RooArgSet domainPars; for (auto const &domainRef : analysis.domains) { auto thisDomain = std::find_if(doc.domains.begin(), doc.domains.end(), diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 5901705b95d39..c180df7f4fe62 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6712,3 +6712,40 @@ TEST(RooFitHS3, ConstraintInModelAndExternal) ASSERT_TRUE(RooJSONFactoryWSTool{ws3}.importJSONfromString(exported2)); EXPECT_EQ(RooJSONFactoryWSTool{ws3}.exportJSONtoString(), exported2); } + +// The parameters that only an external constraint of the ModelConfig depends on keep their roles through a round +// trip: its observable stays a global observable, and its parameter a nuisance parameter. +TEST(RooFitHS3, ExternalConstraintKeepsRoles) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::physics(x[0, -5, 5], mu[0, -2, 2], 1.0)"); + ws.factory("Gaussian::constraint(gobs[0, -5, 5], theta[0, -5, 5], 1.0)"); + ws.var("gobs")->setConstant(true); + RooDataSet data{"data", "data", *ws.var("x")}; + data.add(*ws.var("x")); + ws.import(data, RooFit::Silence()); + RooFit::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("physics"); + mc.SetObservables("x"); + mc.SetParametersOfInterest("mu"); + mc.SetNuisanceParameters("theta"); + mc.SetGlobalObservables("gobs"); + mc.SetExternalConstraints(RooArgSet{*ws.pdf("constraint")}); + ws.import(mc); + + const std::string exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(exported)) << exported; + auto *mc2 = dynamic_cast(ws2.obj("ModelConfig")); + ASSERT_NE(mc2, nullptr); + ASSERT_NE(mc2->GetGlobalObservables(), nullptr) << exported; + ASSERT_NE(mc2->GetNuisanceParameters(), nullptr) << exported; + EXPECT_EQ(mc2->GetGlobalObservables()->size(), 1u) << exported; + EXPECT_NE(mc2->GetGlobalObservables()->find("gobs"), nullptr) << exported; + EXPECT_EQ(mc2->GetNuisanceParameters()->size(), 1u) << exported; + EXPECT_NE(mc2->GetNuisanceParameters()->find("theta"), nullptr) << exported; + ASSERT_NE(mc2->GetExternalConstraints(), nullptr); + EXPECT_NE(mc2->GetExternalConstraints()->find("constraint"), nullptr); +} From 3a87c84112adf72b2a43e951d5365070cc46e92c Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Sat, 14 Nov 2026 23:03:25 +0100 Subject: [PATCH 15/18] [RF][HS3] Write the constraints of channels as terms of the likelihood In RooFit, constraints are factors of the products of the channels, and RooFit counts each constraint once (RooAbsPdf::getAllConstraints()), also if several channels share it. In HS3, a combined_dist is the product of its components, so a constraint shared by two channels would enter the likelihood twice; the export failed because the components share its variate. The constraints that RooFit finds in the channels are now written as terms of the likelihood ("aux_distributions") and removed from the products, whether or not channels share them, as HS3 intends for constraint terms. This covers the factors of the channel products, of the pdf of the ModelConfig itself or of the components of a RooSimultaneous, if no other object uses the product. Constraints elsewhere, e.g. in nested products, stay where they are. HistFactory channels are not changed: their constraints are no factors in the document. On import, a term of the likelihood that is not part of a distribution becomes an external constraint of the ModelConfig, so after a round trip the pdf of the ModelConfig no longer contains these constraints. Reported by Phmonski in https://github.com/root-project/root/pull/24005#issuecomment-6876975913. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/RooJSONFactoryWSTool.cxx | 74 +++++++++++++++- roofit/hs3/test/testRooFitHS3.cxx | 111 ++++++++++++++++++++++++ 2 files changed, 183 insertions(+), 2 deletions(-) diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 0f42f4d822c42..c43f75636690f 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -41,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -985,8 +987,9 @@ void declareBookkeepingColumns(JSONNode &rootnode, RooWorkspace const &ws) // Write each term of the likelihoods of the exported document `rootnode` once. A RooFit model often has a constraint // both as a factor of the product of a channel and as a term of the likelihood (e.g. a constraint of a HistFactory -// modifier, or an external constraint of the ModelConfig); RooFit counts it once. In HS3 it would enter twice, so the -// factor is removed from the product. Only products that no other distribution uses are changed (being a component +// modifier, an external constraint of the ModelConfig, or any constraint of a channel, see +// constraintFactorsOfChannels()); RooFit counts it once. In HS3 it would enter twice, so the factor is removed from +// the product. Only products that no other distribution uses are changed (being a component // of a combined distribution is not a use), and only factors that are terms of every likelihood that has the product // as a term. void writeLikelihoodTermsOnce(JSONNode &rootnode) @@ -3001,6 +3004,61 @@ void RooJSONFactoryWSTool::exportModelConfig(RooFit::ModelConfig const &mc, } } +namespace { + +// The constraint terms of the pdf `pdf` of a ModelConfig with the observables `observables` that are written as +// terms of the likelihood instead of as factors of products: the constraints that RooFit finds +// (RooAbsPdf::getAllConstraints()) and that are factors of a channel, i.e. of `pdf` itself or of a component of the +// RooSimultaneous `pdf`, if the channel is an ordinary product that no other object of the workspace uses. In RooFit, +// constraints are factors of the channels, and RooFit counts each once, also if several channels share it; in HS3, +// they are terms of the likelihood. writeLikelihoodTermsOnce() takes them out of the products. Constraints elsewhere, +// e.g. in a nested product, stay where they are. HistFactory channels (`isHistFactoryChannel`) are skipped: their +// constraints are no factors in the document. +std::vector +constraintFactorsOfChannels(RooWorkspace const &ws, RooAbsPdf const &pdf, RooArgSet const &observables, + std::function const &isHistFactoryChannel) +{ + RooArgSet params; + pdf.getParameters(&observables, params); + std::unique_ptr constraints{pdf.getAllConstraints(observables, params, /*stripDisconnected=*/false)}; + std::vector channels; + if (auto const *simPdf = dynamic_cast(&pdf)) { + for (auto const &item : simPdf->indexCat()) { + if (RooAbsPdf const *component = simPdf->getPdf(item.first)) { + channels.push_back(component); + } + } + } else { + channels.push_back(&pdf); + } + std::vector factors; + for (RooAbsPdf const *channel : channels) { + auto const *prod = dynamic_cast(channel); + if (!prod || isHistFactoryChannel(prod->GetName())) { + continue; + } + // Being a component of a RooSimultaneous, i.e. of a combined distribution, is not a use. + bool usedElsewhere = false; + for (RooAbsArg const *client : prod->clients()) { + if (ws.arg(client->GetName()) == client && !dynamic_cast(client)) { + usedElsewhere = true; + } + } + if (usedElsewhere) { + continue; + } + for (RooAbsArg const *factor : prod->pdfList()) { + if (constraints->find(*factor) && + std::find(factors.begin(), factors.end(), factor->GetName()) == factors.end()) { + factors.push_back(factor->GetName()); + } + } + } + return factors; +} + +} // namespace + void RooJSONFactoryWSTool::exportSingleModelConfig(RooFit::ModelConfig const &mc, std::string const &analysisName, std::string const *dataName) { @@ -3052,6 +3110,18 @@ void RooJSONFactoryWSTool::exportSingleModelConfig(RooFit::ModelConfig const &mc addAux(constr->GetName()); } } + // Without observables (e.g. a ModelConfig without data), RooFit can't tell constraints from other pdfs. + if (RooArgSet const *observables = mc.GetObservables(); observables && !observables->empty()) { + auto isHistFactoryChannel = [&](std::string const &name) { + return std::any_of(doc.distributions.begin(), doc.distributions.end(), [&](HS3::Distribution const &dist) { + return dist.name == name && std::holds_alternative(dist.body); + }); + }; + for (std::string const &factor : + constraintFactorsOfChannels(_workspace, *pdf, *observables, isHistFactoryChannel)) { + addAux(factor); + } + } doc.likelihoods.push_back(std::move(likelihood)); if (RooArgSet const *pois = mc.GetParametersOfInterest()) { diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index c180df7f4fe62..8507d7121b597 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6749,3 +6749,114 @@ TEST(RooFitHS3, ExternalConstraintKeepsRoles) ASSERT_NE(mc2->GetExternalConstraints(), nullptr); EXPECT_NE(mc2->GetExternalConstraints()->find("constraint"), nullptr); } + +// Constraints that RooFit finds in the channels of a model (RooAbsPdf::getAllConstraints()) are written as terms of +// the likelihood, not as factors of the products, also if only one channel has them. A constraint shared by two +// channels is one term of the likelihood in RooFit, and it is written once. A product that another pdf uses keeps its +// constraints. The document reads back with the same NLL, with the constraints as external constraints of the +// ModelConfig, and is stable. +TEST(RooFitHS3, ConstraintFactorsOfChannelsAreLikelihoodTerms) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::a(xa[0, -5, 5], mu[0, -2, 2], 1.0)"); + ws.factory("Gaussian::b(xb[0, -5, 5], expr::mu_theta('mu + theta', mu, theta[0, -5, 5]), 1.0)"); + ws.factory("Gaussian::shared(gobs[0, -5, 5], theta, 1.0)"); + ws.factory("Gaussian::own(gobs_eta[0, -5, 5], eta[0, -5, 5], 1.0)"); + ws.factory("Gaussian::a_shape(xa, eta, 2.0)"); + ws.var("gobs")->setConstant(true); + ws.var("gobs_eta")->setConstant(true); + ws.factory("PROD::channel_a(SUM::a_sum(f[0.5, 0, 1] * a, a_shape), shared, own)"); + ws.factory("PROD::channel_b(b, shared)"); + ws.factory("SIMUL::model(channel[A=0, B=1], A=channel_a, B=channel_b)"); + RooArgSet observables{*ws.var("xa"), *ws.var("xb"), *ws.cat("channel")}; + RooDataSet data{"data", "data", observables}; + for (int index : {0, 1}) { + ws.cat("channel")->setIndex(index); + ws.var("xa")->setVal(0.3); + ws.var("xb")->setVal(-0.6); + data.add(observables); + } + ws.import(data, RooFit::Silence()); + RooFit::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("model"); + mc.SetParametersOfInterest("mu"); + mc.SetNuisanceParameters("theta,eta,f"); + mc.SetObservables(observables); + mc.SetGlobalObservables("gobs,gobs_eta"); + ws.import(mc); + + auto nllOf = [](RooWorkspace &w) { + auto *modelConfig = dynamic_cast(w.obj("ModelConfig")); + if (!modelConfig || !w.data("data")) { + return std::numeric_limits::quiet_NaN(); + } + w.var("theta")->setVal(0.4); + w.var("eta")->setVal(-0.3); + w.var("mu")->setVal(0.2); + RooArgSet external; + if (modelConfig->GetExternalConstraints()) { + external.add(*modelConfig->GetExternalConstraints()); + } + std::unique_ptr nll{ + modelConfig->GetPdf()->createNLL(*w.data("data"), RooFit::ExternalConstraints(external), + RooFit::GlobalObservables(*modelConfig->GetGlobalObservables()))}; + return nll->getVal(); + }; + const double nll = nllOf(ws); + ASSERT_FALSE(std::isnan(nll)); + + const std::string exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(exported); + auto const &distributions = tree->rootnode()["distributions"]; + for (auto const &[channel, factor] : + std::vector>{{"channel_a", "a_sum"}, {"channel_b", "b"}}) { + const auto *product = RooJSONFactoryWSTool::findNamedChild(distributions, channel); + ASSERT_NE(product, nullptr) << exported; + ASSERT_EQ((*product)["factors"].num_children(), 1u) << channel << ": " << exported; + EXPECT_EQ((*product)["factors"].child(0).val(), factor) << exported; + } + const auto *likelihood = + RooJSONFactoryWSTool::findNamedChild(tree->rootnode()["likelihoods"], "ModelConfig_data_likelihood"); + ASSERT_NE(likelihood, nullptr) << exported; + std::set aux; + for (auto const &item : (*likelihood)["aux_distributions"].children()) { + EXPECT_TRUE(aux.insert(item.val()).second) << item.val() << " twice: " << exported; + } + EXPECT_EQ(aux, (std::set{"shared", "own"})) << exported; + + RooWorkspace ws2; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(exported)) << exported; + }); + EXPECT_EQ(warnings.find("is shared by the"), std::string::npos) << warnings; + EXPECT_NEAR(nllOf(ws2), nll, 1e-10 * std::abs(nll)); + const std::string exported2 = RooJSONFactoryWSTool{ws2}.exportJSONtoString(); + RooWorkspace ws3; + ASSERT_TRUE(RooJSONFactoryWSTool{ws3}.importJSONfromString(exported2)); + EXPECT_EQ(RooJSONFactoryWSTool{ws3}.exportJSONtoString(), exported2); + + // A product that another pdf uses keeps its constraint. + RooWorkspace wsUsed; + wsUsed.factory("Gaussian::shape(x[0, -5, 5], theta[0, -5, 5], 1.0)"); + wsUsed.factory("Gaussian::constraint(gobs[0, -5, 5], theta, 1.0)"); + wsUsed.var("gobs")->setConstant(true); + wsUsed.factory("PROD::model(shape, constraint)"); + wsUsed.factory("SUM::other(f[0.5, 0, 1] * model, Uniform::flat(x))"); + RooDataSet dataUsed{"data", "data", *wsUsed.var("x")}; + dataUsed.add(*wsUsed.var("x")); + wsUsed.import(dataUsed, RooFit::Silence()); + RooFit::ModelConfig mcUsed{"ModelConfig", &wsUsed}; + mcUsed.SetPdf("model"); + mcUsed.SetParametersOfInterest("theta"); + mcUsed.SetObservables("x"); + mcUsed.SetGlobalObservables("gobs"); + wsUsed.import(mcUsed); + const std::string exportedUsed = RooJSONFactoryWSTool{wsUsed}.exportJSONtoString(); + auto treeUsed = RooFit::Detail::JSONTree::create(exportedUsed); + const auto *model = RooJSONFactoryWSTool::findNamedChild(treeUsed->rootnode()["distributions"], "model"); + ASSERT_NE(model, nullptr) << exportedUsed; + EXPECT_EQ((*model)["factors"].num_children(), 2u) << exportedUsed; + EXPECT_EQ(exportedUsed.find("aux_distributions"), std::string::npos) << exportedUsed; +} From ce82387e8159026c50eb3f75d6457c8c452bdcab Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Sat, 14 Nov 2026 23:56:15 +0100 Subject: [PATCH 16/18] [RF][HS3] Observables of analyses without data The import took the observables of a ModelConfig only from the data sets that its likelihood pairs with the distributions. For a likelihood without data sets, e.g. written by RooFit for a ModelConfig without data, the ModelConfig had no observables after the import. RooFit now writes the observables of a ModelConfig whose likelihood has no data set in misc.ROOT_internal.ModelConfigs ("observables"), and the import takes them from there. Without that list (documents of other tools, or written before), they are inferred: by the standard, all parameters of the distributions of a likelihood are listed in a domain of the analysis or are constant, so a variate of the distributions of the likelihood that is neither is an observable. Requiring all three excludes the parameters and the observed values of constraint terms. The columns of a combined distribution are observables as well. The import reports inferred observables with an info message. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/HS3Document.h | 2 + roofit/hs3/src/HS3Parser.cxx | 1 + roofit/hs3/src/HS3Writer.cxx | 3 + roofit/hs3/src/RooJSONFactoryWSTool.cxx | 87 ++++++++++++++++- roofit/hs3/test/testRooFitHS3.cxx | 120 ++++++++++++++++++++++++ 5 files changed, 211 insertions(+), 2 deletions(-) diff --git a/roofit/hs3/src/HS3Document.h b/roofit/hs3/src/HS3Document.h index fce5b36ebe835..a68b61394e421 100644 --- a/roofit/hs3/src/HS3Document.h +++ b/roofit/hs3/src/HS3Document.h @@ -468,6 +468,8 @@ struct ModelConfigInfo { std::optional mcName; std::optional pdfName; std::optional combinedDataName; + /// The observables of the ModelConfig, written if no data set of the likelihood gives them. + std::optional> observables; }; struct StepWidth { diff --git a/roofit/hs3/src/HS3Parser.cxx b/roofit/hs3/src/HS3Parser.cxx index 520e928dce1ec..74d263cf02cfa 100644 --- a/roofit/hs3/src/HS3Parser.cxx +++ b/roofit/hs3/src/HS3Parser.cxx @@ -1157,6 +1157,7 @@ void describe(Reader &ar, ModelConfigInfo &info) ar("mcName", info.mcName); ar("pdfName", info.pdfName); ar("combined_data_name", info.combinedDataName); + ar("observables", info.observables); } namespace { diff --git a/roofit/hs3/src/HS3Writer.cxx b/roofit/hs3/src/HS3Writer.cxx index e2ef6649dc759..9498fbde8fba8 100644 --- a/roofit/hs3/src/HS3Writer.cxx +++ b/roofit/hs3/src/HS3Writer.cxx @@ -378,6 +378,9 @@ void writeRooFitInternal(JSONNode &root, RooFitInternal const &internal) if (item.second.combinedDataName) { node["combined_data_name"] << *item.second.combinedDataName; } + if (item.second.observables) { + node["observables"].fill_seq(*item.second.observables); + } } } if (!internal.channelConstraints.empty()) { diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index c43f75636690f..0a90289ffa835 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -1412,9 +1412,63 @@ void checkCombinedPairing(RooSimultaneous const &simPdf, std::string const &data } // Import an analysis (likelihood + domains) as one or more ModelConfig objects into the workspace. +// The observables of the analysis `analysis` if no data set of its likelihood gives them: the list that RooFit writes +// in misc.ROOT_internal.ModelConfigs for a ModelConfig without data, or else the variables that HS3 makes +// observables. By the standard, all parameters of the distributions of the likelihood are listed in a domain of the +// analysis or are constant, so a variate of the distributions of the likelihood (`variates`, see collectVariates()) +// that is neither, is an observable. Requiring all three excludes the parameters, and the observed values of the +// constraint terms, which are constant parameters. +void observablesWithoutData(HS3Document const &doc, HS3::Analysis const &analysis, HS3::ModelConfigInfo const *mcInfo, + std::set const &variates, RooWorkspace &workspace, RooArgSet &observables) +{ + if (mcInfo && mcInfo->observables) { + for (std::string const &name : *mcInfo->observables) { + if (RooAbsArg *arg = workspace.arg(name)) { + observables.add(*arg); + } else { + RooJSONFactoryWSTool::warning("RooFitHS3: the observable '" + name + "' of the analysis '" + analysis.name + + "' (misc.ROOT_internal.ModelConfigs) does not exist"); + } + } + return; + } + std::set inDomains; + for (auto const &domainRef : analysis.domains) { + for (HS3::Domain const &domain : doc.domains) { + if (domain.name != domainRef.name) { + continue; + } + for (auto const &axis : domain.axes) { + inDomains.insert(std::visit([](auto const &a) -> std::string const & { return a.name; }, axis)); + } + } + } + for (auto const &poi : analysis.parametersOfInterest) { + inDomains.insert(poi.name); + } + std::string list; + for (std::string const &variate : variates) { + RooAbsArg *arg = workspace.arg(variate); + if (!dynamic_cast(arg) && !dynamic_cast(arg)) { + continue; + } + if (inDomains.count(variate) || arg->isConstant()) { + continue; + } + observables.add(*arg); + list += (list.empty() ? "'" : ", '") + variate + "'"; + } + if (!list.empty()) { + oocoutI(nullptr, IO) << "RooFitHS3: no data set of the likelihood of the analysis '" << analysis.name + << "' gives its observables; they are the variates of the likelihood that are neither in a " + "domain of the analysis nor constant: " + << list << std::endl; + } +} + void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWorkspace &workspace, const std::vector> &datasets, - std::set const &bookkeepingColumns) + std::set const &bookkeepingColumns, std::set const &variates) { // if this is a toplevel pdf, also create a modelConfig for it std::string const &analysisName = analysis.name; @@ -1494,6 +1548,9 @@ void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWo observables.remove(*arg); } } + if (observables.empty()) { + observablesWithoutData(doc, analysis, mcInfo, variates, workspace, observables); + } // A combined distribution paired with a combined data set must have a sub-data set for every component. for (auto const &term : likelihood.terms) { @@ -3164,6 +3221,15 @@ void RooJSONFactoryWSTool::exportSingleModelConfig(RooFit::ModelConfig const &mc HS3::ModelConfigInfo &info = doc.rooFit.modelConfigs[analysisName]; info.pdfName = pdf->GetName(); info.mcName = mc.GetName(); + // Without a data set, the document doesn't say what the observables are. + if (!dataName && mc.GetObservables() && !mc.GetObservables()->empty()) { + std::vector names; + for (RooAbsArg const *arg : *mc.GetObservables()) { + names.push_back(arg->GetName()); + } + std::sort(names.begin(), names.end()); + info.observables = std::move(names); + } } // Export all top-level pdfs, functions, datasets and ModelConfigs of the workspace into `n`. @@ -3568,7 +3634,24 @@ void RooJSONFactoryWSTool::importAllNodes(JSONNode &n) } for (auto const &analysis : doc.analyses) { - importAnalysis(doc, analysis, _workspace, datasets, bookkeepingColumns); + // The variates of the distributions of the likelihood, see observablesWithoutData(). + std::set variates; + for (auto const &likelihood : doc.likelihoods) { + if (likelihood.name == analysis.likelihood.name) { + for (auto const &term : likelihood.terms) { + std::set const v = variatesOf(n, term.distribution.name); + variates.insert(v.begin(), v.end()); + // The columns of a combined distribution are observables as well. + JSONNode const *dist = findInSection(n, "distributions", term.distribution.name); + if (dist && typeOf(*dist) == "combined_dist") { + if (JSONNode const *columns = dist->find("columns")) { + addNames(*columns, variates); + } + } + } + } + } + importAnalysis(doc, analysis, _workspace, datasets, bookkeepingColumns, variates); } combineDatasets(doc, datasets); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 8507d7121b597..eb5d9530a4050 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6860,3 +6860,123 @@ TEST(RooFitHS3, ConstraintFactorsOfChannelsAreLikelihoodTerms) EXPECT_EQ((*model)["factors"].num_children(), 2u) << exportedUsed; EXPECT_EQ(exportedUsed.find("aux_distributions"), std::string::npos) << exportedUsed; } + +namespace { + +// The names of the observables of the ModelConfig `name` of `ws`, sorted. +std::vector observableNames(RooWorkspace &ws, const char *name) +{ + std::vector names; + auto *mc = dynamic_cast(ws.obj(name)); + if (mc && mc->GetObservables()) { + for (RooAbsArg const *arg : *mc->GetObservables()) { + names.push_back(arg->GetName()); + } + } + std::sort(names.begin(), names.end()); + return names; +} + +// Remove "observables" from misc.ROOT_internal.ModelConfigs of the document `json`. +std::string withoutObservablesList(std::string const &json) +{ + auto tree = RooFit::Detail::JSONTree::create(json); + auto &modelConfigs = tree->rootnode()["misc"]["ROOT_internal"]["ModelConfigs"]; + for (auto &item : modelConfigs.children()) { + if (item.has_child("observables")) { + item.erase("observables"); + } + } + std::stringstream ss; + tree->rootnode().writeJSON(ss); + return ss.str(); +} + +} // namespace + +// The observables of a ModelConfig without data set are not given by the data of its likelihood. RooFit writes them +// in misc.ROOT_internal.ModelConfigs. Without that list, they are the variates of the likelihood that are neither in +// a domain of the analysis nor constant, which HS3 makes observables, and the columns of a combined distribution. +TEST(RooFitHS3, ObservablesWithoutData) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::shape(x[0, -5, 5], mu[0, -2, 2], 1.0)"); + ws.factory("Gaussian::shift(x, theta[0, -5, 5], 2.0)"); + ws.factory("SUM::phys(f[0.5, 0, 1] * shape, shift)"); + ws.factory("Gaussian::constraint(gobs[0, -5, 5], theta, 1.0)"); + ws.var("gobs")->setConstant(true); + ws.factory("PROD::model(phys, constraint)"); + RooFit::ModelConfig mc{"ModelConfig", &ws}; + mc.SetPdf("model"); + mc.SetObservables("x"); + mc.SetParametersOfInterest("mu"); + mc.SetNuisanceParameters("theta,f"); + mc.SetGlobalObservables("gobs"); + ws.import(mc); + + const std::string exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + EXPECT_NE(exported.find(R"("observables":["x"])"), std::string::npos) << exported; + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(exported)) << exported; + EXPECT_EQ(observableNames(ws2, "ModelConfig"), std::vector{"x"}); + // After the first round trip, the constraint is an external constraint, which changes the order of the document. + // From there on, it is stable. + const std::string exported2 = RooJSONFactoryWSTool{ws2}.exportJSONtoString(); + EXPECT_NE(exported2.find(R"("observables":["x"])"), std::string::npos) << exported2; + RooWorkspace ws2b; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2b}.importJSONfromString(exported2)) << exported2; + EXPECT_EQ(RooJSONFactoryWSTool{ws2b}.exportJSONtoString(), exported2); + + // Without the list: inferred, with a message. + const std::string withoutList = withoutObservablesList(exported); + ASSERT_EQ(withoutList.find(R"("observables")"), std::string::npos) << withoutList; + RooWorkspace ws3; + const std::string info = captureMessages(RooFit::INFO, RooFit::IO, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws3}.importJSONfromString(withoutList)) << withoutList; + }); + EXPECT_EQ(observableNames(ws3, "ModelConfig"), std::vector{"x"}); + EXPECT_NE(info.find("neither in a domain of the analysis nor constant: 'x'"), std::string::npos) << info; + + // A RooSimultaneous: the index category is an observable as well. + RooWorkspace wsSim; + wsSim.factory("Gaussian::a(xa[0, -5, 5], mu[0, -2, 2], 1.0)"); + wsSim.factory("Gaussian::b(xb[0, -5, 5], mu, 1.0)"); + wsSim.factory("SIMUL::sim(channel[A=0, B=1], A=a, B=b)"); + RooFit::ModelConfig mcSim{"ModelConfig", &wsSim}; + mcSim.SetPdf("sim"); + mcSim.SetObservables("xa,xb,channel"); + mcSim.SetParametersOfInterest("mu"); + wsSim.import(mcSim); + const std::vector simObservables{"channel", "xa", "xb"}; + const std::string exportedSim = RooJSONFactoryWSTool{wsSim}.exportJSONtoString(); + for (std::string const &json : {exportedSim, withoutObservablesList(exportedSim)}) { + RooWorkspace wsSim2; + ASSERT_TRUE(RooJSONFactoryWSTool{wsSim2}.importJSONfromString(json)) << json; + EXPECT_EQ(observableNames(wsSim2, "ModelConfig"), simObservables) << json; + } + + // A document written by hand, with one domain for all parameters of the analysis. Like RooFit, it has a number + // instead of a data set in the likelihood. + const std::string handWritten = R"({ + "metadata": {"hs3_version": "0.3"}, + "distributions": [ + {"name": "shape", "type": "gaussian_dist", "x": "x", "mean": "mu", "sigma": 1.0}, + {"name": "constraint", "type": "gaussian_dist", "x": "gobs", "mean": "theta", "sigma": 1.0}, + {"name": "model", "type": "product_dist", "factors": ["shape"]} + ], + "domains": [ + {"name": "parameters", "type": "product_domain", + "axes": [{"name": "mu", "min": -2.0, "max": 2.0}, {"name": "theta", "min": -5.0, "max": 5.0}]}, + {"name": "default_domain", "type": "product_domain", "axes": [{"name": "x", "min": -5.0, "max": 5.0}]} + ], + "parameter_points": [{"name": "default_values", "parameters": [ + {"name": "mu", "value": 0.0}, {"name": "theta", "value": 0.0}, {"name": "gobs", "value": 0.0, "const": true}]}], + "likelihoods": [{"name": "lh", "distributions": ["model"], "data": [0], "aux_distributions": ["constraint"]}], + "analyses": [{"name": "analysis", "likelihood": "lh", "parameters_of_interest": ["mu"], "domains": ["parameters"]}] +})"; + RooWorkspace ws4; + ASSERT_TRUE(RooJSONFactoryWSTool{ws4}.importJSONfromString(handWritten)); + EXPECT_EQ(observableNames(ws4, "analysis"), std::vector{"x"}); +} From 26c2c7aa6ffab6065ac3fbc6189a2d82828ae090 Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 12 Nov 2026 20:59:10 +0100 Subject: [PATCH 17/18] [RF][HS3] Read and write category_conditional_dist HS3 #147 (split out of #119) defines the "category_conditional_dist": one of several sub-distributions, selected by the value of categorical parameters and conditional on it. RooFit has this as the "switch mode" of RooSimultaneous: with an index category that is not an observable, it evaluates to the selected component, and its expected number of events is the one of that component. Import: a category_conditional_dist becomes a RooSimultaneous over its categorical parameters, which must be declared by categorical axes of a domain like the columns of a combined_dist. Export: RooFit doesn't distinguish the two uses of a RooSimultaneous by the object, only by the normalization set. A combined distribution is the top-level pdf of a model, while a conditional one (e.g. a choice between alternative background shapes) is a building block of a model. A RooSimultaneous that is used by another pdf is therefore written as category_conditional_dist, with "categorical_parameter" instead of "columns", and one that no other pdf uses as combined_dist. This is a heuristic until RooFit has a more explicit way to mark the conditional use. A category_conditional_dist that no other distribution uses is read with a warning that it will be written back as a combined_dist. The pairing checks of combined_dist in a likelihood use the type in the document. The value of a RooCategory in a snapshot is written to the parameter point as the label, and the category to the default domain; the columns of combined distributions are not parameters and are left out. For the check of distinct variates (see "Distinct variates of likelihood terms and combined components"), a category_conditional_dist has the variates of its alternatives, which may share them. A RooSimultaneous that no other pdf uses is written as combined_dist, so if its alternatives share an observable the export fails; the import warning for an unused category_conditional_dist says so. Test: CategoryConditionalDist. Ported onto the typed HS3 document: the value of a categorical parameter is a label of the HS3::ParameterValue, the columns of combined distributions are kept in the export state, and the pairing check and the import warning look up the type of the generic distribution in the document. Assisted-by: claude-code:claude-fable-5-1 Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- roofit/hs3/src/JSONFactories_RooFitCore.cxx | 19 ++- roofit/hs3/src/RooJSONFactoryWSTool.cxx | 87 +++++++++-- roofit/hs3/test/testRooFitHS3.cxx | 159 ++++++++++++++++++++ 3 files changed, 249 insertions(+), 16 deletions(-) diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index 1725355ac39b7..5d1a14fcb16cb 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -423,17 +423,23 @@ bool importAddition(RooJSONFactoryWSTool *tool, const JSONNode &p) } // A "combined_dist" is a RooSimultaneous. Its index category is the single column, or a RooSuperCategory over the -// columns in the order of the document; the columns are categories declared by categorical axes of the domains. -bool importCombinedDist(RooJSONFactoryWSTool *tool, const JSONNode &p) +// columns in the order of the document; the columns are categories declared by categorical axes of the domains. A +// "category_conditional_dist" is the same over its categorical parameters (RooFit evaluates a RooSimultaneous whose +// index category is not an observable as the selected component). On export, the two are told apart by whether the +// RooSimultaneous is used by another pdf, see RooJSONFactoryWSTool::exportSimultaneous(). +template +bool importSimultaneous(RooJSONFactoryWSTool *tool, const JSONNode &p) { std::string const name(RooJSONFactoryWSTool::name(p)); - std::string const context = "the combined_dist '" + name + "'"; + const char *type = Conditional ? "category_conditional_dist" : "combined_dist"; + std::string const context = "the " + std::string(type) + " '" + name + "'"; // Whether a combined distribution is extended follows from its components; HS3 forbids declaring it. - if (p.has_child("extended")) { + if (!Conditional && p.has_child("extended")) { RooJSONFactoryWSTool::error(context + " must not have \"extended\": whether it is extended follows from its " + "components"); } - std::vector columns = readSelectionColumns(*tool->workspace(), p, "columns", context); + std::vector columns = + readSelectionColumns(*tool->workspace(), p, Conditional ? "categorical_parameter" : "columns", context); auto const *componentsNode = p.find("components"); if (!componentsNode || !componentsNode->is_seq() || componentsNode->num_children() == 0) { @@ -1691,7 +1697,8 @@ STATIC_EXECUTE([]() { registerImporter("sum", false); registerImporter("mixture_dist", false); registerImporter("mixture_model", false); - registerImporter("combined_dist", false); + registerImporter>("combined_dist", false); + registerImporter>("category_conditional_dist", false); registerImporter("mixture_resolution_model", false); registerImporter("binsampling_dist", false); registerImporter("binsampling", false); diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 0a90289ffa835..e8604b11e2915 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -164,6 +164,9 @@ struct RooFit::HS3::ToolExportState { std::map> domains; /// The categories that are declared by categorical axes of the "default_domain", by name. std::map categories; + /// The columns of the exported combined distributions: they are not parameters, and don't appear in parameter + /// points. + std::set columnCategories; /// Set by the exporter of an object that is interpreted by the document. std::optional typedObject; std::vector channelConstraints; @@ -667,8 +670,9 @@ std::vector combinedComponents(JSONNode const &node, const char *fi } // The variates of the distribution `name` of the document: the "x" of a fundamental distribution (a name or a list) -// and the axes of a distribution with axes (e.g. histfactory_dist), through products, mixtures and combined -// distributions. The constraint terms named by a histfactory_dist are terms of their own, see likelihoodTerms(). +// and the axes of a distribution with axes (e.g. histfactory_dist), through products, mixtures, combined and +// category-conditional distributions. The constraint terms named by a histfactory_dist are terms of their own, see +// likelihoodTerms(). void collectVariates(JSONNode const &rootnode, std::string const &name, std::set &out, std::set &visited) { @@ -700,7 +704,8 @@ void collectVariates(JSONNode const &rootnode, std::string const &name, std::set recurse(namesIn("factors")); } else if (type == "mixture_dist") { recurse(namesIn("summands")); - } else if (type == "combined_dist") { + } else if (type == "combined_dist" || type == "category_conditional_dist") { + // The alternatives of a category_conditional_dist may share variates: only one of them is selected. recurse(combinedComponents(*node, "dist")); } else { if (JSONNode const *x = node->find("x")) { @@ -1347,6 +1352,22 @@ void requirePdf(RooAbsArg const *arg, std::string const &what) // adds the expected number of events of an extended component to the likelihood. That changes the fit, so it is an // error (https://github.com/root-project/root/issues/24004). For a non-extended component it changes nothing, and a // sub-data set without component is ignored by RooFit; both are reported as warnings. +// Whether a RooSimultaneous is a "category_conditional_dist" rather than a "combined_dist". RooFit doesn't tell the +// two apart by the object, only by the normalization set: the index category is an observable for a combined +// distribution and a parameter for a conditional one. A combined distribution is the top-level pdf of a model, while +// a conditional one (e.g. a choice between alternative background shapes) is a building block of a model. A +// RooSimultaneous that is used by another pdf is therefore taken as conditional. This is a heuristic, until RooFit +// has a more explicit way to mark the conditional use. +bool isCategoryConditional(RooSimultaneous const &simPdf) +{ + for (RooAbsArg const *client : simPdf.clients()) { + if (dynamic_cast(client)) { + return true; + } + } + return false; +} + void checkCombinedPairing(RooSimultaneous const &simPdf, std::string const &dataName, HS3::CombinedData const &data) { std::vector columns = selectionColumns(simPdf.indexCat()); @@ -1552,11 +1573,21 @@ void importAnalysis(HS3Document const &doc, HS3::Analysis const &analysis, RooWo observablesWithoutData(doc, analysis, mcInfo, variates, workspace, observables); } - // A combined distribution paired with a combined data set must have a sub-data set for every component. + // A combined distribution paired with a combined data set must have a sub-data set for every component. A + // category_conditional_dist is also a RooSimultaneous, but it is not split by the data. + auto isConditional = [&](std::string const &name) { + for (HS3::Distribution const &dist : doc.distributions) { + if (dist.name == name) { + auto const *generic = std::get_if(&dist.body); + return generic && generic->type == "category_conditional_dist"; + } + } + return false; + }; for (auto const &term : likelihood.terms) { auto const *dataRef = std::get_if(&term.data); auto *simPdf = dynamic_cast(workspace.pdf(term.distribution.name)); - if (!dataRef || !simPdf) { + if (!dataRef || !simPdf || isConditional(term.distribution.name)) { continue; } for (HS3::Data const &data : doc.data) { @@ -2057,9 +2088,21 @@ RooAbsReal *RooJSONFactoryWSTool::requestImpl(const std::string &obj return nullptr; } -// Export a single variable (RooRealVar or RooConstVar) `v` to the parameter point. +// Export a single variable (RooRealVar, RooConstVar or RooCategory) `v` to the parameter point. void RooJSONFactoryWSTool::exportVariable(const RooAbsArg *v, HS3::ParameterPoint &point, bool storeConstant) { + // A categorical parameter: its value is the label. The columns of combined distributions are not parameters. + if (auto *cat = dynamic_cast(v)) { + if (_export->columnCategories.count(cat->GetName()) > 0) { + return; + } + HS3::ParameterValue var; + var.name = cat->GetName(); + var.value = std::string{cat->getCurrentLabel()}; + recordCategory(*cat); + point.parameters.push_back(std::move(var)); + return; + } auto *cv = dynamic_cast(v); auto *rrv = dynamic_cast(v); if (!cv && !rrv) @@ -2333,24 +2376,33 @@ void RooJSONFactoryWSTool::exportObject(RooAbsArg const &func, std::set &exportedObjectNames) { RooAbsCategory const &indexCat = simPdf.indexCat(); std::vector columns = selectionColumns(indexCat); + const bool conditional = isCategoryConditional(simPdf); JSONNode &elem = appendNamedChild((*_export->root)["distributions"], simPdf.GetName()); - elem["type"] << "combined_dist"; - auto &columnsNode = elem["columns"].set_seq(); + elem["type"] << (conditional ? "category_conditional_dist" : "combined_dist"); + auto &columnsNode = elem[conditional ? "categorical_parameter" : "columns"].set_seq(); for (RooCategory const *column : columns) { columnsNode.append_child() << column->GetName(); recordCategory(*column); + if (!conditional) { + _export->columnCategories.insert(column->GetName()); + } } auto &components = elem["components"].set_seq(); // Components with renamed observables replace the original ones (Config::renameCombinedObservables). @@ -3034,7 +3086,7 @@ void RooJSONFactoryWSTool::exportModelConfig(RooFit::ModelConfig const &mc, auto pdf = mc.GetPdf(); auto simpdf = dynamic_cast(pdf); int founddata = 0; - if (simpdf) { + if (simpdf && !isCategoryConditional(*simpdf)) { // One analysis per combined data set with the same columns. std::vector columns; for (RooCategory const *column : selectionColumns(simpdf->indexCat())) { @@ -3552,6 +3604,21 @@ void RooJSONFactoryWSTool::importAllNodes(JSONNode &n) // This creates the index categories, which the parameter points can refer to. combinePdfs(doc, _workspace); + // RooFit tells a category_conditional_dist apart from a combined_dist only by whether another pdf uses it (see + // isCategoryConditional()). One that no other pdf uses would be written back as a combined_dist. + for (auto const &dist : doc.distributions) { + auto const *generic = std::get_if(&dist.body); + if (!generic || generic->type != "category_conditional_dist") { + continue; + } + auto *simPdf = dynamic_cast(_workspace.pdf(dist.name)); + if (simPdf && !isCategoryConditional(*simPdf)) { + warning("RooFitHS3: the category_conditional_dist '" + dist.name + + "' is not used by another distribution. RooFit can't tell it apart from a combined_dist, and will " + "export it as a combined_dist, which fails if its components share variates."); + } + } + for (auto const &point : doc.parameterPoints) { RooJSONFactoryWSTool::testValidName(point.name, true); diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index eb5d9530a4050..fc37c30a58e71 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -6094,6 +6094,165 @@ TEST(RooFitHS3, CombinedDataPairing) } } +// A "category_conditional_dist" (HS3 #147) selects a component by the value of categorical parameters. It is a +// RooSimultaneous whose index category is a parameter: RooFit then evaluates the selected component. RooFit doesn't +// distinguish it from a combined_dist by the object; a RooSimultaneous used by another pdf is written as a +// category_conditional_dist, and the value of the parameter is written to the parameter points. +TEST(RooFitHS3, CategoryConditionalDist) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + const std::string json = R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "x", "min": 0.0, "max": 10.0}, {"name": "frac", "min": 0.0, "max": 1.0}, + {"name": "bkg_function", "categories": ["narrow", "wide"]}]}], + "parameter_points": [{"name": "point", "parameters": [{"name": "bkg_function", "value": "wide"}, + {"name": "frac", "value": 0.3}]}], + "distributions": [ + {"name": "narrow", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 1.0}, + {"name": "wide", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 3.0}, + {"name": "bkg", "type": "category_conditional_dist", "categorical_parameter": ["bkg_function"], + "components": [{"key": ["narrow"], "dist": "narrow"}, {"key": ["wide"], "dist": "wide"}]}, + {"name": "sig", "type": "gaussian_dist", "x": "x", "mean": 2.0, "sigma": 0.5}, + {"name": "model", "type": "mixture_dist", "summands": ["sig", "bkg"], "coefficients": ["frac"]}] + })"; + RooWorkspace ws; + const std::string importWarnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{ws}.importJSONfromString(json)); + }); + EXPECT_EQ(importWarnings.find("category_conditional_dist"), std::string::npos) << importWarnings; + auto *bkg = dynamic_cast(ws.pdf("bkg")); + ASSERT_NE(bkg, nullptr); + RooCategory *choice = ws.cat("bkg_function"); + ASSERT_NE(choice, nullptr); + EXPECT_EQ(&bkg->indexCat(), choice); + + // The value is the one of the selected component, conditional on the parameter (not in the normalization set). + RooRealVar &x = *ws.var("x"); + RooArgSet nset{x}; + ASSERT_TRUE(ws.loadSnapshot("point")); + EXPECT_STREQ(choice->getCurrentLabel(), "wide"); + for (double value : {2.0, 5.0, 8.0}) { + x.setVal(value); + choice->setLabel("wide"); + EXPECT_DOUBLE_EQ(bkg->getVal(nset), ws.pdf("wide")->getVal(nset)) << "x = " << value; + choice->setLabel("narrow"); + EXPECT_DOUBLE_EQ(bkg->getVal(nset), ws.pdf("narrow")->getVal(nset)) << "x = " << value; + } + + // Export: the same type, the parameter in the domain and in the parameter point, and a stable document. + ws.loadSnapshot("point"); + const std::string exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(exported); + auto const &root = tree->rootnode(); + auto const *bkgNode = RooJSONFactoryWSTool::findNamedChild(root["distributions"], "bkg"); + ASSERT_NE(bkgNode, nullptr) << exported; + EXPECT_EQ((*bkgNode)["type"].val(), "category_conditional_dist"); + ASSERT_EQ((*bkgNode)["categorical_parameter"].num_children(), 1u); + EXPECT_EQ((*bkgNode)["categorical_parameter"].child(0).val(), "bkg_function"); + EXPECT_FALSE(bkgNode->has_child("columns")); + auto const *point = RooJSONFactoryWSTool::findNamedChild(root["parameter_points"], "point"); + ASSERT_NE(point, nullptr) << exported; + auto const *choiceInPoint = RooJSONFactoryWSTool::findNamedChild((*point)["parameters"], "bkg_function"); + ASSERT_NE(choiceInPoint, nullptr) << exported; + EXPECT_EQ((*choiceInPoint)["value"].val(), "wide"); + auto const *domain = RooJSONFactoryWSTool::findNamedChild(root["domains"], "default_domain"); + ASSERT_NE(domain, nullptr); + EXPECT_NE(RooJSONFactoryWSTool::findNamedChild((*domain)["axes"], "bkg_function"), nullptr) << exported; + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(exported)); + ws2.loadSnapshot("point"); + EXPECT_EQ(RooJSONFactoryWSTool{ws2}.exportJSONtoString(), exported); + + // A RooSimultaneous built in RooFit is a combined_dist if no other pdf uses it, and a category_conditional_dist + // otherwise. As a combined_dist, alternatives over the same observable can't be written: its components must have + // distinct variates. + { + RooWorkspace wsRooFit; + wsRooFit.factory("Gaussian::narrow(x[0, 10], 5.0, 1.0)"); + wsRooFit.factory("Gaussian::wide(x, 5.0, 3.0)"); + wsRooFit.factory("SIMUL::bkg(bkg_function[narrow=0, wide=1], narrow=narrow, wide=wide)"); + expectExportThrowsWithError( + wsRooFit, "the variate 'x' is shared by the components 'narrow' and 'wide' of the combined_dist " + "'bkg'"); + wsRooFit.factory("Gaussian::sig(x, 2.0, 0.5)"); + wsRooFit.factory("SUM::model(frac[0.3, 0, 1] * sig, bkg)"); + const std::string exportedRooFit = RooJSONFactoryWSTool{wsRooFit}.exportJSONtoString(); + EXPECT_NE(exportedRooFit.find(R"("type":"category_conditional_dist")"), std::string::npos) << exportedRooFit; + EXPECT_EQ(exportedRooFit.find(R"("type":"combined_dist")"), std::string::npos) << exportedRooFit; + } + + // A category_conditional_dist that no other distribution uses can't be told apart from a combined_dist: the + // import warns that it will be written back as one. + { + const std::string jsonTopLevel = R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "x", "min": 0.0, "max": 10.0}, {"name": "bkg_function", "categories": ["narrow", "wide"]}]}], + "distributions": [ + {"name": "narrow", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 1.0}, + {"name": "wide", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 3.0}, + {"name": "bkg", "type": "category_conditional_dist", "categorical_parameter": ["bkg_function"], + "components": [{"key": ["narrow"], "dist": "narrow"}, {"key": ["wide"], "dist": "wide"}]}] + })"; + RooWorkspace wsTopLevel; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [&] { + ASSERT_TRUE(RooJSONFactoryWSTool{wsTopLevel}.importJSONfromString(jsonTopLevel)); + }); + EXPECT_NE(warnings.find("the category_conditional_dist 'bkg' is not used by another distribution"), + std::string::npos) + << warnings; + EXPECT_NE(warnings.find("which fails if its components share variates"), std::string::npos) << warnings; + expectExportThrowsWithError(wsTopLevel, "the variate 'x' is shared by the components 'narrow' and 'wide'"); + } + + // In a likelihood, a category_conditional_dist has the variates of its alternatives: here, "x" is a variate of + // the term "model" only through them. + { + const std::string withLikelihood = R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [ + {"name": "x", "min": 0.0, "max": 10.0}, {"name": "y", "min": 0.0, "max": 10.0}, + {"name": "bkg_function", "categories": ["narrow", "wide"]}]}], + "distributions": [ + {"name": "narrow", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 1.0}, + {"name": "wide", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 3.0}, + {"name": "bkg", "type": "category_conditional_dist", "categorical_parameter": ["bkg_function"], + "components": [{"key": ["narrow"], "dist": "narrow"}, {"key": ["wide"], "dist": "wide"}]}, + {"name": "sig", "type": "gaussian_dist", "x": "y", "mean": 2.0, "sigma": 0.5}, + {"name": "model", "type": "product_dist", "factors": ["sig", "bkg"]}, + {"name": "other", "type": "gaussian_dist", "x": "x", "mean": 4.0, "sigma": 1.0}], + "data": [{"name": "data", "type": "unbinned", + "axes": [{"name": "x", "min": 0.0, "max": 10.0}, {"name": "y", "min": 0.0, "max": 10.0}], + "entries": [[5.0, 2.0]]}], + "likelihoods": [{"name": "lh", "distributions": ["model"], "data": ["data"], "aux_distributions": AUX}], + "analyses": [{"name": "an", "likelihood": "lh", "domains": ["default_domain"]}] + })"; + auto withAux = [&](std::string const &aux) { + std::string json = withLikelihood; + json.replace(json.find("AUX"), 3, aux); + return json; + }; + RooWorkspace wsLikelihood; + EXPECT_TRUE(RooJSONFactoryWSTool{wsLikelihood}.importJSONfromString(withAux("[]"))); + expectImportThrowsWithError(withAux(R"(["other"])"), + "the variate 'x' is shared by the terms 'model' and 'other' of the likelihood 'lh'"); + } + + // The parameter must be declared by a categorical axis of a domain. + expectImportThrowsWithError(R"({ + "metadata": {"hs3_version": "0.3"}, + "domains": [{"name": "default_domain", "type": "product_domain", "axes": [{"name": "x", "min": 0.0, "max": 10.0}]}], + "distributions": [ + {"name": "narrow", "type": "gaussian_dist", "x": "x", "mean": 5.0, "sigma": 1.0}, + {"name": "bkg", "type": "category_conditional_dist", "categorical_parameter": ["bkg_function"], + "components": [{"key": ["narrow"], "dist": "narrow"}]}] + })", + "the column 'bkg_function' of the category_conditional_dist 'bkg' is not declared by a " + "categorical axis of a domain"); +} + // The names of a RooSimultaneous and of a combined data set are part of the standard document now, not of // misc.ROOT_internal: a document without "misc" imports with the same names, and the ModelConfig finds its pdf // through the likelihood. From 9309dff2f18eebbef386ad4ae652be4fd39d076e Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Tue, 10 Nov 2026 11:10:59 +0100 Subject: [PATCH 18/18] [RF][HS3] Write RooMultiPdf as category_conditional_dist A RooMultiPdf (discrete profiling, e.g. in CMS Combine) couldn't be exported ("no export keys available"). It is now written as a category_conditional_dist: its category is the categorical parameter, and the key of the pdf at list position i is the label of the state with index i, since a RooMultiPdf selects by index. A pdf without a state of its index is an error. The penalty term that a RooMultiPdf adds to the likelihood (a correction for the number of free parameters of the selected pdf) has no HS3 representation yet. It is dropped with a warning, and its constants are not written. The category_conditional_dist is read back as a RooSimultaneous, which evaluates to the same pdf. Test: MultiPdfExport. Ported onto the typed HS3 document: the category is declared through recordCategory(), and the distribution is recorded as a generic object of the exported document. Assisted-by: claude-code:claude-opus-5-5 Co-Authored-By: Claude Opus 5.5 --- .../hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h | 2 + roofit/hs3/src/RooJSONFactoryWSTool.cxx | 50 +++++++++++++++ roofit/hs3/test/testRooFitHS3.cxx | 64 +++++++++++++++++++ 3 files changed, 116 insertions(+) diff --git a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h index 5f52793695d9d..27667af2645c7 100644 --- a/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h +++ b/roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h @@ -48,6 +48,7 @@ class ModelConfig; class RooRealVar; class RooAbsCategory; class RooCategory; +class RooMultiPdf; class RooSimultaneous; class RooJSONFactoryWSTool { @@ -260,6 +261,7 @@ class RooJSONFactoryWSTool { void exportSimultaneous(RooSimultaneous const &simPdf, std::set &exportedObjectNames); void recordCategory(RooAbsCategory const &cat); + void exportMultiPdf(RooMultiPdf const &multiPdf, std::set &exportedObjectNames); void exportData(RooAbsData const &data); RooJSONFactoryWSTool::CombinedData exportCombinedData(RooAbsData const &data); diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index e8604b11e2915..86a0410ca9696 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -2248,6 +2249,9 @@ void RooJSONFactoryWSTool::exportObject(RooAbsArg const &func, std::set(&func)) { exportSimultaneous(*simPdf, exportedObjectNames); return; + } else if (auto multiPdf = dynamic_cast(&func)) { + exportMultiPdf(*multiPdf, exportedObjectNames); + return; } else if (dynamic_cast(&func)) { // Categories are not objects of the document: the index category of a RooSimultaneous is declared by the // categorical axes of the domain that its "columns" name. @@ -2460,6 +2464,52 @@ void RooJSONFactoryWSTool::recordCategory(RooAbsCategory const &cat) _export->categories[axis.name] = std::move(axis); } +/** + * @brief Export a RooMultiPdf as a "category_conditional_dist". + * + * A RooMultiPdf evaluates to the pdf at the list position equal to the index of its category. The key of the pdf at + * position i is therefore the label of the state with index i. The category is a categorical parameter, declared by a + * categorical axis of the default domain; its value is written to the parameter points. + * + * The penalty term that a RooMultiPdf adds to the likelihood (a correction for the number of free parameters of the + * selected pdf) has no HS3 representation yet, and is dropped with a warning. The category_conditional_dist is read + * back as a RooSimultaneous, so the class doesn't survive a round trip. + */ +void RooJSONFactoryWSTool::exportMultiPdf(RooMultiPdf const &multiPdf, std::set &exportedObjectNames) +{ + std::string const name = multiPdf.GetName(); + auto const *indexCat = dynamic_cast(&multiPdf.indexCategory().arg()); + if (!indexCat) { + error("cannot export the RooMultiPdf '" + name + "' as a category_conditional_dist: its index '" + + multiPdf.indexCategory().arg().GetName() + "' is not a RooCategory"); + } + + JSONNode &elem = appendNamedChild((*_export->root)["distributions"], name); + elem["type"] << "category_conditional_dist"; + elem["categorical_parameter"].set_seq().append_child() << indexCat->GetName(); + recordCategory(*indexCat); + auto &components = elem["components"].set_seq(); + std::vector pdfs; + for (int i = 0; i < multiPdf.getNumPdfs(); ++i) { + if (!indexCat->hasIndex(i)) { + error("cannot export the RooMultiPdf '" + name + + "' as a category_conditional_dist: it has a pdf at position " + std::to_string(i) + + ", but its category '" + indexCat->GetName() + "' has no state with that index"); + } + RooAbsPdf const *pdf = multiPdf.getPdf(i); + JSONNode &component = components.append_child().set_map(); + component["key"].set_seq().append_child() << indexCat->lookupName(i); + component["dist"] << pdf->GetName(); + pdfs.push_back(pdf); + } + warning("RooFitHS3: the RooMultiPdf '" + name + + "' is written as a category_conditional_dist without its penalty term (the correction to the likelihood " + "for the number of free parameters of the selected pdf), which HS3 can't represent yet. It is read back as " + "a RooSimultaneous."); + recordGenericObject(_export->doc, true, name, elem); + exportObjects(pdfs, exportedObjectNames); +} + /** * @brief Import a function from the JSONNode into the workspace. * diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index fc37c30a58e71..f8e49c53252d5 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -6253,6 +6254,69 @@ TEST(RooFitHS3, CategoryConditionalDist) "categorical axis of a domain"); } +// A RooMultiPdf (discrete profiling, e.g. in CMS Combine) is written as a category_conditional_dist: the key of the +// pdf at position i is the label of the state with index i. Its penalty term has no HS3 representation yet and is +// dropped with a warning. It is read back as a RooSimultaneous that evaluates to the same pdf. +TEST(RooFitHS3, MultiPdfExport) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooWorkspace ws; + ws.factory("Gaussian::narrow(x[0, 10], 5.0, 1.0)"); + ws.factory("Exponential::falling(x, c[-0.5, -2, 0])"); + RooCategory pdfIndex{"pdf_index", "pdf_index"}; + RooMultiPdf multiPdf{"bkg", "bkg", pdfIndex, RooArgList{*ws.pdf("narrow"), *ws.pdf("falling")}}; + ws.import(multiPdf, RooFit::Silence()); + ws.factory("Gaussian::sig(x, 2.0, 0.5)"); + ws.factory("SUM::model(frac[0.3, 0, 1] * sig, bkg)"); + ws.cat("pdf_index")->setIndex(1); + ws.saveSnapshot("point", RooArgSet{*ws.cat("pdf_index")}); + + std::string exported; + const std::string warnings = captureMessages(RooFit::WARNING, RooFit::ObjectHandling, + [&] { exported = RooJSONFactoryWSTool{ws}.exportJSONtoString(); }); + EXPECT_NE(warnings.find("the RooMultiPdf 'bkg' is written as a category_conditional_dist without its penalty term"), + std::string::npos) + << warnings; + + auto tree = RooFit::Detail::JSONTree::create(exported); + auto const &root = tree->rootnode(); + auto const *bkgNode = RooJSONFactoryWSTool::findNamedChild(root["distributions"], "bkg"); + ASSERT_NE(bkgNode, nullptr) << exported; + EXPECT_EQ((*bkgNode)["type"].val(), "category_conditional_dist"); + ASSERT_EQ((*bkgNode)["categorical_parameter"].num_children(), 1u); + EXPECT_EQ((*bkgNode)["categorical_parameter"].child(0).val(), "pdf_index"); + ASSERT_EQ((*bkgNode)["components"].num_children(), 2u) << exported; + EXPECT_EQ((*bkgNode)["components"].child(0)["key"].child(0).val(), "_pdf0"); + EXPECT_EQ((*bkgNode)["components"].child(0)["dist"].val(), "narrow"); + EXPECT_EQ((*bkgNode)["components"].child(1)["key"].child(0).val(), "_pdf1"); + EXPECT_EQ((*bkgNode)["components"].child(1)["dist"].val(), "falling"); + // The penalty constants of the RooMultiPdf are not written. + EXPECT_EQ(exported.find("constnarrow"), std::string::npos) << exported; + + RooWorkspace ws2; + ASSERT_TRUE(RooJSONFactoryWSTool{ws2}.importJSONfromString(exported)); + auto *bkg2 = dynamic_cast(ws2.pdf("bkg")); + ASSERT_NE(bkg2, nullptr); + ASSERT_TRUE(ws2.loadSnapshot("point")); + EXPECT_STREQ(ws2.cat("pdf_index")->getCurrentLabel(), "_pdf1"); + RooRealVar &x = *ws.var("x"); + RooRealVar &x2 = *ws2.var("x"); + RooArgSet nset{x}; + RooArgSet nset2{x2}; + for (int index : {0, 1}) { + ws.cat("pdf_index")->setIndex(index); + ws2.cat("pdf_index")->setLabel(ws.cat("pdf_index")->getCurrentLabel()); + for (double value : {2.0, 5.0, 8.0}) { + x.setVal(value); + x2.setVal(value); + EXPECT_NEAR(bkg2->getVal(nset2), ws.pdf("bkg")->getVal(nset), 1e-12) << "index " << index << ", x = " << value; + EXPECT_NEAR(ws2.pdf("model")->getVal(nset2), ws.pdf("model")->getVal(nset), 1e-12) + << "index " << index << ", x = " << value; + } + } +} + // The names of a RooSimultaneous and of a combined data set are part of the standard document now, not of // misc.ROOT_internal: a document without "misc" imports with the same names, and the ModelConfig finds its pdf // through the likelihood.