From 91328cdeb56b2995f1b1fac9f7bc7410415de813 Mon Sep 17 00:00:00 2001 From: Phmonski Date: Wed, 22 Jul 2026 16:35:02 +0200 Subject: [PATCH] [HS3] Fix export of self_normalized flag --- roofit/hs3/src/JSONFactories_RooFitCore.cxx | 8 +- roofit/hs3/test/testRooFitHS3.cxx | 82 +++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/roofit/hs3/src/JSONFactories_RooFitCore.cxx b/roofit/hs3/src/JSONFactories_RooFitCore.cxx index 80c9e7afdc63b..5c6a93e6f13e7 100644 --- a/roofit/hs3/src/JSONFactories_RooFitCore.cxx +++ b/roofit/hs3/src/JSONFactories_RooFitCore.cxx @@ -1016,7 +1016,11 @@ bool importWrapperPdf(RooJSONFactoryWSTool *tool, const JSONNode &node) bool selfNormalized = false; - if (auto sn = node.find("selfNormalized")) + auto sn = node.find("self_normalized"); + // ROOT previously exported this key without an underscore. + if (!sn) + sn = node.find("selfnormalized"); + if (sn) selfNormalized = sn->val_bool(); tool->wsEmplace(name, *func, selfNormalized); @@ -1039,7 +1043,7 @@ bool exportWrapperPdf(RooJSONFactoryWSTool *, const RooAbsArg *arg, JSONNode &no node["function"] << funcProxy->absArg()->GetName(); if (pdf->selfNormalized()) - node["selfnormalized"] << true; + node["self_normalized"] << true; return true; } diff --git a/roofit/hs3/test/testRooFitHS3.cxx b/roofit/hs3/test/testRooFitHS3.cxx index 2f2a71b7835c5..f146998fe1d98 100644 --- a/roofit/hs3/test/testRooFitHS3.cxx +++ b/roofit/hs3/test/testRooFitHS3.cxx @@ -35,6 +35,7 @@ #include #include #include +#include #include #include @@ -212,6 +213,17 @@ std::size_t countOccurrences(std::string_view haystack, std::string_view needle) return result; } +std::string makeWrapperPdfJson(bool selfNormalized) +{ + RooRealVar x{"x", "x", 0.5, 0.0, 1.0}; + RooFormulaVar function{"function", "1.0 + 0.0 * x", RooArgList{x}}; + RooWrapperPdf pdf{"pdf", "pdf", function, selfNormalized}; + + RooWorkspace workspace; + workspace.import(pdf, RooFit::Silence()); + return RooJSONFactoryWSTool{workspace}.exportJSONtoString(); +} + // Asserts that exporting `ws` to HS3 throws and logs an error message containing `expectedReason`. void expectExportThrowsWithError(RooWorkspace &ws, std::string const &expectedReason) { @@ -802,6 +814,76 @@ TEST(RooFitHS3, RooGenericPdf) EXPECT_EQ(status, 0); } +TEST(RooFitHS3, RooWrapperPdfSelfNormalizedRoundTrip) +{ + const std::string json = makeWrapperPdfJson(true); + EXPECT_NE(json.find("\"self_normalized\":true"), std::string::npos) << json; + EXPECT_EQ(json.find("\"selfnormalized\""), std::string::npos) << json; + EXPECT_EQ(json.find("\"selfNormalized\""), std::string::npos) << json; + + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + auto *pdf = dynamic_cast(imported.pdf("pdf")); + ASSERT_NE(pdf, nullptr); + EXPECT_TRUE(pdf->selfNormalized()); + + const std::string defaultJson = makeWrapperPdfJson(false); + EXPECT_EQ(defaultJson.find("self_normalized"), std::string::npos) << defaultJson; + + RooWorkspace importedDefault; + ASSERT_TRUE(RooJSONFactoryWSTool{importedDefault}.importJSONfromString(defaultJson)); + auto *defaultPdf = dynamic_cast(importedDefault.pdf("pdf")); + ASSERT_NE(defaultPdf, nullptr); + EXPECT_FALSE(defaultPdf->selfNormalized()); +} + +TEST(RooFitHS3, RooWrapperPdfSelfNormalizedLegacyCompatibility) +{ + std::string legacyJson = makeWrapperPdfJson(true); + const auto canonicalPos = legacyJson.find("self_normalized"); + ASSERT_NE(canonicalPos, std::string::npos) << legacyJson; + legacyJson.replace(canonicalPos, std::string{"self_normalized"}.size(), "selfnormalized"); + + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(legacyJson)); + auto *pdf = dynamic_cast(imported.pdf("pdf")); + ASSERT_NE(pdf, nullptr); + EXPECT_TRUE(pdf->selfNormalized()); + + const std::string canonicalJson = RooJSONFactoryWSTool{imported}.exportJSONtoString(); + EXPECT_NE(canonicalJson.find("\"self_normalized\":true"), std::string::npos) << canonicalJson; + EXPECT_EQ(canonicalJson.find("\"selfnormalized\""), std::string::npos) << canonicalJson; +} + +TEST(RooFitHS3, RooWrapperPdfSelfNormalizedCanonicalKeyTakesPrecedence) +{ + std::string json = makeWrapperPdfJson(true); + const std::string canonicalField = "\"self_normalized\":true"; + const auto canonicalPos = json.find(canonicalField); + ASSERT_NE(canonicalPos, std::string::npos) << json; + json.replace(canonicalPos, canonicalField.size(), "\"self_normalized\":false,\"selfnormalized\":true"); + + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + auto *pdf = dynamic_cast(imported.pdf("pdf")); + ASSERT_NE(pdf, nullptr); + EXPECT_FALSE(pdf->selfNormalized()); +} + +TEST(RooFitHS3, RooWrapperPdfSelfNormalizedCamelCaseKeyIsIgnored) +{ + std::string json = makeWrapperPdfJson(true); + const auto canonicalPos = json.find("self_normalized"); + ASSERT_NE(canonicalPos, std::string::npos) << json; + json.replace(canonicalPos, std::string{"self_normalized"}.size(), "selfNormalized"); + + RooWorkspace imported; + ASSERT_TRUE(RooJSONFactoryWSTool{imported}.importJSONfromString(json)); + auto *pdf = dynamic_cast(imported.pdf("pdf")); + ASSERT_NE(pdf, nullptr); + EXPECT_FALSE(pdf->selfNormalized()); +} + TEST(RooFitHS3, GenericExpressionCleanup) { RooRealVar x{"x", "x", 0.5, -1.0, 1.0};