From 9d290b06868c8f1b307cfe0e20ffc2c827a7e5dd Mon Sep 17 00:00:00 2001 From: Carsten Burgard Date: Thu, 5 Nov 2026 14:34:42 +0100 Subject: [PATCH] [RF][HS3] Export RooParamHistFunc as a product of two step functions A RooParamHistFunc with relative parameters (the default) takes in bin i the value p_i times the content of bin i of its histogram. This is what a HistFactory sample does, so it is now written like one: as a `product` of a `step` with the histogram (RooHistFunc, `_nominal`) and a `step` with the parameters (ParamHistFunc, `_scale`). It is read back as a RooProduct of these two, with the same values and integral. RooParamHistFunc with absolute parameters (paramRelative = false) is not supported and raises an error: its analytical integral is not consistent with its value. The observable must have the binning of the histogram. Assisted-by: claude-code:claude-opus-5-5 --- roofit/hs3/src/JSONFactories_RooFitCore.cxx | 54 +++++++++++++ roofit/hs3/src/RooJSONFactoryWSTool.cxx | 12 ++- roofit/hs3/test/testRooFitHS3.cxx | 86 +++++++++++++++++++++ 3 files changed, 149 insertions(+), 3 deletions(-) diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index ce7102885bc66..0bd3bec177012 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -1338,6 +1339,56 @@ bool exportParamHistFunc(RooJSONFactoryWSTool *, const RooAbsArg *func, JSONNode return true; } +// Whether two binnings have the same bins. +bool sameBins(RooAbsBinning const &a, RooAbsBinning const &b) +{ + if (a.numBins() != b.numBins()) { + return false; + } + for (int i = 0; i < a.numBins(); ++i) { + if (a.binLow(i) != b.binLow(i) || a.binHigh(i) != b.binHigh(i)) { + return false; + } + } + return true; +} + +// RooParamHistFunc (relative parameters): the value in bin i is p_i times the content of bin i of its histogram. This +// is written like a HistFactory sample, as the product of a "step" with the histogram (RooHistFunc) and a "step" with +// the parameters (ParamHistFunc). It is read back as a RooProduct of these two. +bool exportRooParamHistFunc(RooJSONFactoryWSTool *tool, const RooAbsArg *func, JSONNode &elem, std::string const &key) +{ + auto const *phf = static_cast(func); + const std::string name = phf->GetName(); + if (!phf->relParam()) { + RooJSONFactoryWSTool::error("RooParamHistFunc '" + name + + "' with absolute parameters (paramRelative = false) can't be exported to HS3: " + "only relative parameters are supported"); + } + if (phf->xList().size() != 1) { + RooJSONFactoryWSTool::error("RooParamHistFunc '" + name + "' must have exactly one observable"); + } + auto *x = dynamic_cast(phf->xList().at(0)); + RooDataHist const &dh = phf->dataHist(); + if (!x || !sameBins(x->getBinning(), *dh.getBinnings()[0])) { + RooJSONFactoryWSTool::error("RooParamHistFunc '" + name + + "': the binning of its observable differs from the binning of its histogram"); + } + + auto nominal = + std::make_unique((name + "_nominal").c_str(), (name + "_nominal").c_str(), RooArgSet{*x}, dh); + auto scale = std::make_unique((name + "_scale").c_str(), (name + "_scale").c_str(), RooArgList{*x}, + phf->paramList()); + + elem["type"] << key; + auto &factors = elem["factors"].set_seq(); + factors.append_child() << nominal->GetName(); + factors.append_child() << scale->GetName(); + tool->queueExportTemporary(nominal.release()); + tool->queueExportTemporary(scale.release()); + return true; +} + bool exportSpline(RooJSONFactoryWSTool *, const RooAbsArg *func, JSONNode &elem, std::string const &key) { auto const *rs = static_cast(func); @@ -1523,6 +1574,9 @@ STATIC_EXECUTE([]() { registerExporter(RooFFTConvPdf::Class(), "fft_convolution_dist", false); registerExporter(RooExtendPdf::Class(), "rate_extended_dist", false); registerExporter(ParamHistFunc::Class(), "step", false); + // The dependants are the two temporary step functions queued by the exporter. + registerExporter(RooParamHistFunc::Class(), "product", false, + /*autoExportDependants=*/false); registerExporter(RooSpline::Class(), "spline", false); }); diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 9f372161b8e7c..d4bb4ae0508db 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -825,8 +825,10 @@ void checkHS3Version(std::string const &version) } } -/// HS3 requires the names of all top-level objects to be unique within a document. Throw if a name is used twice, or -/// only warn if not `strict`: RooFit wrote such documents before HS3 v0.3, e.g. for a range and a pdf of one name. +/// HS3 requires the names of all top-level objects to be unique within a document. If `strict`, throw if a name is +/// used twice within one section or by a distribution and a function. Other clashes, and all clashes if not +/// `strict`, only warn: RooFit's own export still produces them, e.g. for a range and a pdf of one name, and it +/// must be able to read its own output. void checkUniqueNames(JSONNode const &rootnode, bool strict) { std::map sectionOfName; @@ -850,7 +852,11 @@ void checkUniqueNames(JSONNode const &rootnode, bool strict) // compatibility: analyses are not referenced by name, and likelihoods only from analyses, so there is no // ambiguity. auto isLegacyNamed = [](std::string const &s) { return s == "likelihoods" || s == "analyses"; }; - if (!strict || isLegacyNamed(firstSection) || isLegacyNamed(section)) { + // Within one section, and between distributions and functions (both can be referenced by name), a clash is + // ambiguous. Other clashes are produced by RooFit itself (a range, snapshot or data set named like a pdf). + auto isReferable = [](std::string const &s) { return s == "distributions" || s == "functions"; }; + const bool ambiguous = firstSection == section || (isReferable(firstSection) && isReferable(section)); + if (!strict || !ambiguous || isLegacyNamed(firstSection) || isLegacyNamed(section)) { RooJSONFactoryWSTool::warning("RooFitHS3: the name '" + name + "' is used in \"" + firstSection + "\" and in \"" + section + "\", which is not allowed by the HS3 standard."); continue; diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 560aea148a51b..38619147e7143 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -4029,3 +4030,88 @@ TEST(RooFitHS3, WarningsAreShownByDefault) captureMessages(RooFit::WARNING, RooFit::ObjectHandling, [] { RooJSONFactoryWSTool::warning("test message"); }); EXPECT_NE(warnings.find("test message"), std::string::npos) << warnings; } + +// A RooParamHistFunc with relative parameters (value in bin i = p_i times the content of bin i) is written like a +// HistFactory sample: a product of a "step" with the histogram and a "step" with the parameters. It is read back as a +// RooProduct of a RooHistFunc and a ParamHistFunc, with the same values and integral. Absolute parameters are not +// supported. +TEST(RooFitHS3, RooParamHistFuncAsHistFactoryProduct) +{ + RooHelpers::LocalChangeMsgLevel changeMsgLvl(RooFit::WARNING); + + RooRealVar x{"x", "x", 0.0, 2.0}; + x.setBins(2); + RooDataHist dh{"dh", "dh", x}; + x.setBin(0); + dh.set(x, 2.0); + x.setBin(1); + dh.set(x, 3.0); + + RooParamHistFunc func{"func", "func", dh, x}; + static_cast(func.paramList()[1]).setVal(1.5); + const std::vector expected{2.0 * 1.0, 3.0 * 1.5}; + for (int i = 0; i < 2; ++i) { + x.setBin(i); + ASSERT_DOUBLE_EQ(func.getVal(), expected[i]); + } + std::unique_ptr integral{func.createIntegral(x)}; + const double expectedIntegral = integral->getVal(); + + RooWorkspace ws; + ws.import(func, RooFit::Silence()); + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + auto tree = RooFit::Detail::JSONTree::create(json); + auto const &functions = tree->rootnode()["functions"]; + auto const *product = RooJSONFactoryWSTool::findNamedChild(functions, "func"); + ASSERT_NE(product, nullptr) << json; + EXPECT_EQ((*product)["type"].val(), "product"); + ASSERT_EQ((*product)["factors"].num_children(), 2u); + EXPECT_EQ((*product)["factors"].child(0).val(), "func_nominal"); + EXPECT_EQ((*product)["factors"].child(1).val(), "func_scale"); + auto const *nominal = RooJSONFactoryWSTool::findNamedChild(functions, "func_nominal"); + ASSERT_NE(nominal, nullptr) << json; + EXPECT_TRUE(nominal->has_child("data")) << json; + auto const *scale = RooJSONFactoryWSTool::findNamedChild(functions, "func_scale"); + ASSERT_NE(scale, nullptr) << json; + EXPECT_TRUE(scale->has_child("parameters")) << json; + + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + auto *importedFunc = dynamic_cast(imported.function("func")); + ASSERT_NE(importedFunc, nullptr); + EXPECT_NE(dynamic_cast(imported.function("func_nominal")), nullptr); + EXPECT_NE(dynamic_cast(imported.function("func_scale")), nullptr); + RooRealVar &xImported = *imported.var("x"); + for (int i = 0; i < 2; ++i) { + xImported.setBin(i); + EXPECT_DOUBLE_EQ(importedFunc->getVal(), expected[i]) << "bin " << i; + } + std::unique_ptr importedIntegral{importedFunc->createIntegral(xImported)}; + EXPECT_NEAR(importedIntegral->getVal(), expectedIntegral, 1e-12); + + // From the second export on, the JSON is stable. + const std::string json2 = RooJSONFactoryWSTool{imported}.exportJSONtoString(); + RooWorkspace imported2; + ASSERT_TRUE(RooJSONFactoryWSTool{imported2}.importJSONfromString(json2)); + EXPECT_EQ(RooJSONFactoryWSTool{imported2}.exportJSONtoString(), json2); + + // Absolute parameters are not supported. + RooParamHistFunc absolute{"absolute", "absolute", dh, x, nullptr, /*paramRelative=*/false}; + RooWorkspace wsAbsolute; + wsAbsolute.import(absolute, RooFit::Silence()); + const std::string errors = captureMessages(RooFit::ERROR, RooFit::IO, [&] { + EXPECT_THROW(RooJSONFactoryWSTool{wsAbsolute}.exportJSONtoString(), std::runtime_error); + }); + EXPECT_NE(errors.find("with absolute parameters (paramRelative = false) can't be exported"), std::string::npos) + << errors; + + // The bins of the observable must be those of the histogram. + RooParamHistFunc otherBinning{"other_binning", "other_binning", dh, x}; + RooWorkspace wsOtherBinning; + wsOtherBinning.import(otherBinning, RooFit::Silence()); + wsOtherBinning.var("x")->setBins(4); + const std::string binningErrors = captureMessages(RooFit::ERROR, RooFit::IO, [&] { + EXPECT_THROW(RooJSONFactoryWSTool{wsOtherBinning}.exportJSONtoString(), std::runtime_error); + }); + EXPECT_NE(binningErrors.find("differs from the binning of its histogram"), std::string::npos) << binningErrors; +}