diff --git a/roofit/hs3/src/JSONFactories_HistFactory.cxx b/roofit/hs3/src/JSONFactories_HistFactory.cxx index a762c7328d61e..cc0d670ee99a8 100644 --- a/roofit/hs3/src/JSONFactories_HistFactory.cxx +++ b/roofit/hs3/src/JSONFactories_HistFactory.cxx @@ -1230,8 +1230,8 @@ bool normSysSupportsMultiplicativeMerge(int interpolationCode) // single modifier. The shared metadata (constraint, parameter and interpolation code) must be identical across the // duplicates; the type-specific `combine` callable performs the actual merge and any additional validation. template -void mergeDuplicateModifiers(const Channel &channel, const Sample &sample, Modifiers &modifiers, - std::string_view type, CombineFn combine) +void mergeDuplicateModifiers(const Channel &channel, const Sample &sample, Modifiers &modifiers, std::string_view type, + CombineFn combine) { Modifiers mergedModifiers; mergedModifiers.reserve(modifiers.size()); @@ -1269,38 +1269,34 @@ void mergeDuplicateModifiers(const Channel &channel, const Sample &sample, Modif void mergeDuplicateNormSys(const Channel &channel, Sample &sample) { - mergeDuplicateModifiers(channel, sample, sample.normsys, "normsys", - [&](NormSys &merged, const NormSys &modifier) { - if (!normSysSupportsMultiplicativeMerge(merged.interpolationCode)) { - duplicateModifierError( - channel, sample, "normsys", merged.name, - "multiplicative combination is only valid for log-space interpolation codes"); - } - merged.low *= modifier.low; - merged.high *= modifier.high; - }); + mergeDuplicateModifiers(channel, sample, sample.normsys, "normsys", [&](NormSys &merged, const NormSys &modifier) { + if (!normSysSupportsMultiplicativeMerge(merged.interpolationCode)) { + duplicateModifierError(channel, sample, "normsys", merged.name, + "multiplicative combination is only valid for log-space interpolation codes"); + } + merged.low *= modifier.low; + merged.high *= modifier.high; + }); } void mergeDuplicateHistoSys(const Channel &channel, Sample &sample) { const std::size_t nBins = sample.hist.size(); - mergeDuplicateModifiers(channel, sample, sample.histosys, "histosys", - [&](HistoSys &merged, const HistoSys &modifier) { - if (merged.interpolationCode != 4) { - duplicateModifierError( - channel, sample, "histosys", merged.name, - "non-default interpolation cannot currently be represented by the HS3 exporter"); - } - if (merged.low.size() != nBins || merged.high.size() != nBins || - modifier.low.size() != nBins || modifier.high.size() != nBins) { - duplicateModifierError(channel, sample, "histosys", merged.name, - "histogram binning differs"); - } - for (std::size_t bin = 0; bin < nBins; ++bin) { - merged.low[bin] += modifier.low[bin] - sample.hist[bin]; - merged.high[bin] += modifier.high[bin] - sample.hist[bin]; - } - }); + mergeDuplicateModifiers( + channel, sample, sample.histosys, "histosys", [&](HistoSys &merged, const HistoSys &modifier) { + if (merged.interpolationCode != 4) { + duplicateModifierError(channel, sample, "histosys", merged.name, + "non-default interpolation cannot currently be represented by the HS3 exporter"); + } + if (merged.low.size() != nBins || merged.high.size() != nBins || modifier.low.size() != nBins || + modifier.high.size() != nBins) { + duplicateModifierError(channel, sample, "histosys", merged.name, "histogram binning differs"); + } + for (std::size_t bin = 0; bin < nBins; ++bin) { + merged.low[bin] += modifier.low[bin] - sample.hist[bin]; + merged.high[bin] += modifier.high[bin] - sample.hist[bin]; + } + }); } void ensureUniqueModifiers(const Channel &channel, const Sample &sample) diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index 80c9e7afdc63b..a098444669e21 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -470,7 +470,7 @@ bool importExponential(RooJSONFactoryWSTool *tool, const JSONNode &p) // }, // { // "name": "c_exponential_inverted", // transformation function created on-the-fly on export - // "type": "generic_function", + // "type": "generic", // "expression": "-c" // } // @@ -1140,7 +1140,7 @@ STATIC_EXECUTE([]() { registerExporter>(RooPolyVar::Class(), "polynomial", false); registerExporter(RooRealSumFunc::Class(), "weighted_sum", false); registerExporter(RooRealSumPdf::Class(), "weighted_sum_dist", false); - registerExporter(RooTFnBinding::Class(), "generic_function", false); + registerExporter(RooTFnBinding::Class(), "generic", false); registerExporter(RooRealIntegral::Class(), "integral", false); registerExporter(RooDerivative::Class(), "derivative", false); registerExporter(RooFFTConvPdf::Class(), "fft_convolution_dist", false); diff --git a/roofit/hs3/src/RooJSONFactoryWSTool.cxx b/roofit/hs3/src/RooJSONFactoryWSTool.cxx index 2a2a7e026bd76..c28e474078839 100644 --- a/roofit/hs3/src/RooJSONFactoryWSTool.cxx +++ b/roofit/hs3/src/RooJSONFactoryWSTool.cxx @@ -942,7 +942,7 @@ std::string RooJSONFactoryWSTool::exportTransformed(const RooAbsReal *original, { std::string newname = std::string(original->GetName()) + suffix; RooFit::Detail::JSONNode &trafo_node = appendNamedChild((*_rootnodeOutput)["functions"], newname); - trafo_node["type"] << "generic_function"; + trafo_node["type"] << "generic"; trafo_node["expression"] << TString::Format(formula.c_str(), original->GetName()).Data(); this->setAttribute(newname, "roofit_skip"); // this function should not be imported back in return newname; diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 2f2a71b7835c5..73a4e535bcac4 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -42,6 +43,7 @@ #include #include +#include #include #include @@ -802,11 +804,36 @@ TEST(RooFitHS3, RooGenericPdf) EXPECT_EQ(status, 0); } +TEST(RooFitHS3, GenericTypeNames) +{ + RooRealVar x{"x", "x", 0.5, -1.0, 1.0}; + RooRealVar y{"y", "y", 0.5, -1.0, 1.0}; + RooRealVar z{"z", "z", 0.5, -1.0, 1.0}; + RooRealVar c{"c", "c", -0.1}; + RooFormulaVar formula{"formula", "x + y", RooArgList{x, y}}; + RooGenericPdf genericPdf{"genericPdf", "x + 2.0", RooArgList{x}}; + TF3 tf3{"tf3", "x + y + z", -1.0, 1.0, -1.0, 1.0, -1.0, 1.0}; + RooTFnBinding binding{"binding", "binding", &tf3, RooArgList{x, y, z}}; + RooExponential exponential{"exponential", "exponential", x, c}; + + RooWorkspace ws{"ws_generic_types"}; + ws.import(formula, RooFit::Silence()); + ws.import(genericPdf, RooFit::Silence(), RooFit::RecycleConflictNodes()); + ws.import(binding, RooFit::Silence(), RooFit::RecycleConflictNodes()); + ws.import(exponential, RooFit::Silence(), RooFit::RecycleConflictNodes()); + + const std::string json = RooJSONFactoryWSTool{ws}.exportJSONtoString(); + EXPECT_NE(json.find("\"name\":\"formula\",\"type\":\"generic\""), std::string::npos) << json; + EXPECT_NE(json.find("\"name\":\"genericPdf\",\"type\":\"generic_dist\""), std::string::npos) << json; + EXPECT_NE(json.find("\"name\":\"binding\",\"type\":\"generic\""), std::string::npos) << json; + EXPECT_NE(json.find("\"name\":\"c_exponential_inverted\",\"type\":\"generic\""), std::string::npos) << json; + EXPECT_EQ(json.find("\"type\":\"generic_function\""), std::string::npos) << json; +} + TEST(RooFitHS3, GenericExpressionCleanup) { RooRealVar x{"x", "x", 0.5, -1.0, 1.0}; - RooFormulaVar formula{"formula", - "formula", + RooFormulaVar formula{"formula", "formula", "TMath::Floor(x) + TMath::Ceil(x) + TMath::Abs(x) + TMath::Tan(x) + " "TMath::ASin(x / 2.) + TMath::ACos(x / 2.) + TMath::ATan(x) + TMath::Pi() + TMath::E()", RooArgList{x}}; @@ -1470,9 +1497,8 @@ TEST(RooFitHS3, HistFactoryDuplicateModifiersAreCombined) ASSERT_TRUE(RooJSONFactoryWSTool{ws1}.importJSONfromString(jsonStr)); std::string exported; - const std::string warnings = captureMessages(RooFit::WARNING, RooFit::IO, [&] { - exported = RooJSONFactoryWSTool{ws1}.exportJSONtoString(); - }); + const std::string warnings = + captureMessages(RooFit::WARNING, RooFit::IO, [&] { exported = RooJSONFactoryWSTool{ws1}.exportJSONtoString(); }); EXPECT_NE(warnings.find("combined 2 duplicate modifiers named 'norm' of type 'normsys'"), std::string::npos) << warnings; @@ -1550,9 +1576,8 @@ TEST(RooFitHS3, HistFactoryDuplicateModifiersAreCombined) } // The operation must be logged as a warning, not as an error. - const std::string sentinelErrors = captureMessages(RooFit::ERROR, RooFit::IO, [] { - RooJSONFactoryWSTool::warning("duplicate-modifier warning-level sentinel"); - }); + const std::string sentinelErrors = captureMessages( + RooFit::ERROR, RooFit::IO, [] { RooJSONFactoryWSTool::warning("duplicate-modifier warning-level sentinel"); }); EXPECT_TRUE(sentinelErrors.empty()) << sentinelErrors; }