[RF][HS3] Preserve explicit binnings for RooGenericPdf and RooFormulaVar

roofit/CMakeLists.txt

@@ -19,7 +19,6 @@ if(mathmore)
 endif()
 add_subdirectory(roostats)
 add_subdirectory(histfactory)
-add_subdirectory(jsoninterface)
 add_subdirectory(hs3)
 if(roofit_legacy_eval_backend AND NOT MSVC)
   add_subdirectory(xroofit)

roofit/histfactory/test/CMakeLists.txt

@@ -13,7 +13,7 @@ if(NOT MSVC OR win_broken_tests)
 endif()
 
 ROOT_ADD_GTEST(testHistFactory testHistFactory.cxx
-  LIBRARIES RooFitCore RooFit RooStats HistFactory RooFitHS3 RooFitJSONInterface
+  LIBRARIES RooFitCore RooFit RooStats HistFactory RooFitHS3
   COPY_TO_BUILDDIR ${CMAKE_CURRENT_SOURCE_DIR}/ref_6.16_example_UsingC_channel1_meas_model.root ${CMAKE_CURRENT_SOURCE_DIR}/ref_6.16_example_UsingC_combined_meas_model.root)
 
 if(clad)

roofit/hs3/CMakeLists.txt

@@ -11,10 +11,12 @@
 
 ROOT_STANDARD_LIBRARY_PACKAGE(RooFitHS3
   HEADERS
+    RooFit/Detail/JSONInterface.h
     RooFitHS3/JSONIO.h
     RooFitHS3/RooJSONFactoryWSTool.h
   SOURCES
     src/Domains.cxx
+    src/JSONInterface.cxx
     src/JSONIO.cxx
     src/RooJSONFactoryWSTool.cxx
     src/JSONFactories_RooFitCore.cxx
@@ -26,7 +28,8 @@ ROOT_STANDARD_LIBRARY_PACKAGE(RooFitHS3
     HistFactory
     RooFit
     RooFitCore
-    RooFitJSONInterface
 )
 
+target_link_libraries(RooFitHS3 PRIVATE nlohmann_json::nlohmann_json)
+
 ROOT_ADD_TEST_SUBDIRECTORY(test)

roofit/jsoninterface/inc/RooFit/Detail/JSONInterface.h → roofit/hs3/inc/RooFit/Detail/JSONInterface.h

@@ -86,7 +86,6 @@ class JSONNode {
 
 public:
    virtual void writeJSON(std::ostream &os) const = 0;
-   virtual void writeYML(std::ostream &) const { throw std::runtime_error("YML not supported"); }
 
 public:
    virtual JSONNode &operator<<(std::string const &s) = 0;
@@ -101,6 +100,7 @@ class JSONNode {
    virtual bool is_map() const = 0;
    virtual bool is_seq() const = 0;
    virtual bool is_null() const = 0;
+   virtual bool is_number() const;
    virtual JSONNode &set_map() = 0;
    virtual JSONNode &set_seq() = 0;
    virtual JSONNode &set_null() = 0;

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 <class Modifiers, class CombineFn>
-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)

roofit/hs3/src/JSONFactories_RooFitCore.cxx

@@ -41,6 +41,7 @@
 #include <RooPoisson.h>
 #include <RooPolynomial.h>
 #include <RooPolyVar.h>
+#include <RooAbsRealLValue.h>
 #include <RooRealSumFunc.h>
 #include <RooRealSumPdf.h>
 #include <RooRealVar.h>
@@ -52,6 +53,7 @@
 #include <RooWorkspace.h>
 #include <RooRealIntegral.h>
 #include <RooSpline.h>
+#include <RooUniformBinning.h>
 #include <TSpline.h>
 
 #include <TF1.h>
@@ -63,8 +65,12 @@
 
 #include <algorithm>
 #include <cctype>
+#include <cmath>
+#include <limits>
+#include <memory>
 #include <set>
 #include <string_view>
+#include <vector>
 
 using RooFit::Detail::JSONNode;
 
@@ -163,6 +169,109 @@ void translateImportedExpression(TString &expr)
    replaceIdentifier(expr, "EULER", "TMath::E()");
 }
 
+int readPositiveInteger(const JSONNode &node, const std::string &context)
+{
+   // Read through val_double() so an integer encoded as a JSON float (e.g. 1e6,
+   // whose textual form is "1e+06") is accepted like elsewhere in HS3, while
+   // fractional, non-finite, out-of-range or non-numeric values are rejected.
+   const double value = node.is_number() ? node.val_double() : std::numeric_limits<double>::quiet_NaN();
+   if (!std::isfinite(value) || value < 1.0 || value != std::floor(value) ||
+       value > static_cast<double>(std::numeric_limits<int>::max())) {
+      RooJSONFactoryWSTool::error("\"nbins\" in " + context + " must be a positive integer");
+   }
+   return static_cast<int>(value);
+}
+
+std::unique_ptr<RooAbsBinning>
+readFormulaAxisBinning(const JSONNode &axis, const std::string &axisName, const std::string &formulaName)
+{
+   const std::string context = "axis '" + axisName + "' of generic formula '" + formulaName + "'";
+   const bool hasEdges = axis.has_child("edges");
+   const bool hasMin = axis.has_child("min");
+   const bool hasMax = axis.has_child("max");
+   const bool hasNBins = axis.has_child("nbins");
+
+   if (hasEdges && (hasMin || hasMax || hasNBins)) {
+      RooJSONFactoryWSTool::error(context + " must use either \"edges\" or \"min\"/\"max\"/\"nbins\"");
+   }
+
+   if (hasEdges) {
+      const JSONNode &edgesNode = axis["edges"];
+      if (!edgesNode.is_seq()) {
+         RooJSONFactoryWSTool::error("\"edges\" in " + context + " must be a sequence");
+      }
+
+      std::vector<double> edges;
+      edges.reserve(edgesNode.num_children());
+      for (const JSONNode &edgeNode : edgesNode.children()) {
+         if (!edgeNode.is_number()) {
+            RooJSONFactoryWSTool::error("\"edges\" in " + context + " must contain only finite values");
+         }
+         const double edge = edgeNode.val_double();
+         if (!std::isfinite(edge)) {
+            RooJSONFactoryWSTool::error("\"edges\" in " + context + " must contain only finite values");
+         }
+         if (!edges.empty() && edge <= edges.back()) {
+            RooJSONFactoryWSTool::error("\"edges\" in " + context + " must be strictly increasing");
+         }
+         edges.push_back(edge);
+      }
+      if (edges.size() < 2) {
+         RooJSONFactoryWSTool::error("\"edges\" in " + context + " must contain at least two values");
+      }
+      return std::make_unique<RooBinning>(static_cast<int>(edges.size() - 1), edges.data());
+   }
+
+   if (!hasMin || !hasMax || !hasNBins) {
+      RooJSONFactoryWSTool::error(context + " must define \"min\", \"max\", and \"nbins\"");
+   }
+
+   if (!axis["min"].is_number() || !axis["max"].is_number()) {
+      RooJSONFactoryWSTool::error("\"min\" and \"max\" in " + context + " must be finite and increasing");
+   }
+   const double min = axis["min"].val_double();
+   const double max = axis["max"].val_double();
+   if (!std::isfinite(min) || !std::isfinite(max) || max <= min) {
+      RooJSONFactoryWSTool::error("\"min\" and \"max\" in " + context + " must be finite and increasing");
+   }
+   return std::make_unique<RooUniformBinning>(min, max, readPositiveInteger(axis["nbins"], context));
+}
+
+template <class RooArg_t>
+void importFormulaBinnings(RooArg_t &arg, const JSONNode &node)
+{
+   if (!node.has_child("axes")) {
+      return;
+   }
+
+   const JSONNode &axes = node["axes"];
+   if (!axes.is_seq()) {
+      RooJSONFactoryWSTool::error("\"axes\" in generic formula '" + std::string(arg.GetName()) +
+                                  "' must be a sequence");
+   }
+
+   std::set<std::string> axisNames;
+   for (const JSONNode &axis : axes.children()) {
+      if (!axis.is_map() || !axis.has_child("name")) {
+         RooJSONFactoryWSTool::error("each axis in generic formula '" + std::string(arg.GetName()) +
+                                     "' must be a map with a \"name\"");
+      }
+      const std::string axisName = axis["name"].val();
+      if (!axisNames.insert(axisName).second) {
+         RooJSONFactoryWSTool::error("duplicate axis '" + axisName + "' in generic formula '" + arg.GetName() + "'");
+      }
+
+      auto *observable = dynamic_cast<RooAbsRealLValue *>(arg.getParameter(axisName.c_str()));
+      if (!observable) {
+         RooJSONFactoryWSTool::error(
+            "axis '" + axisName + "' is not a real-valued formula variable of generic formula '" + arg.GetName() + "'");
+      }
+
+      std::unique_ptr<RooAbsBinning> binning = readFormulaAxisBinning(axis, axisName, arg.GetName());
+      arg.setBinning(*observable, *binning, /*checkFlatness=*/false);
+   }
+}
+
 template <class RooArg_t>
 bool importFormulaArg(RooJSONFactoryWSTool *tool, const JSONNode &p)
 {
@@ -176,7 +285,9 @@ bool importFormulaArg(RooJSONFactoryWSTool *tool, const JSONNode &p)
    for (const auto &d : extractArguments(formula.Data())) {
       dependents.add(*tool->request<RooAbsReal>(d, name));
    }
-   tool->wsImport(RooArg_t{name.c_str(), formula, dependents});
+   RooArg_t arg{name.c_str(), formula, dependents};
+   importFormulaBinnings(arg, p);
+   tool->wsImport(arg);
    return true;
 }
 
@@ -774,6 +885,25 @@ bool exportFormulaArg(RooJSONFactoryWSTool *, const RooAbsArg *func, JSONNode &e
       expression.ReplaceAll(("@" + std::to_string(idx)).c_str(), par->GetName());
    }
    elem["expression"] << expression.Data();
+
+   for (const RooAbsArg *dependent : pdf->dependents()) {
+      auto const *observable = dynamic_cast<const RooAbsRealLValue *>(dependent);
+      if (!observable) {
+         continue;
+      }
+      const RooAbsBinning *binning = pdf->getBinning(*observable);
+      if (!binning) {
+         continue;
+      }
+
+      auto &axes = elem["axes"];
+      if (!axes.is_seq()) {
+         axes.set_seq();
+      }
+      auto &axis = axes.append_child().set_map();
+      axis["name"] << observable->GetName();
+      writeAxisBinning(axis, *binning);
+   }
    return true;
 }
 

roofit/hs3/src/JSONIOUtils.cxx

@@ -1,5 +1,7 @@
 #include "JSONIOUtils.h"
 
+#include <RooAbsBinning.h>
+
 #include <string>
 
 using RooFit::Detail::JSONNode;
@@ -82,3 +84,19 @@ std::unique_ptr<RooFit::Detail::JSONTree> varJSONString(const JSONNode &treeRoot
 
    return jsonDict;
 }
+
+void writeAxisBinning(JSONNode &node, const RooAbsBinning &binning)
+{
+   if (binning.isUniform()) {
+      node["min"] << binning.lowBound();
+      node["max"] << binning.highBound();
+      node["nbins"] << binning.numBins();
+      return;
+   }
+
+   auto &edges = node["edges"].set_seq();
+   edges.append_child() << binning.binLow(0);
+   for (int i = 0; i < binning.numBins(); ++i) {
+      edges.append_child() << binning.binHigh(i);
+   }
+}

roofit/hs3/src/JSONIOUtils.h

@@ -4,9 +4,12 @@
 #include <string_view>
 #include <RooFit/Detail/JSONInterface.h>
 
+class RooAbsBinning;
+
 bool startsWith(std::string_view str, std::string_view prefix);
 bool endsWith(std::string_view str, std::string_view suffix);
 std::string removeSuffix(std::string_view str, std::string_view suffix);
 std::unique_ptr<RooFit::Detail::JSONTree> varJSONString(const RooFit::Detail::JSONNode &treeRoot);
+void writeAxisBinning(RooFit::Detail::JSONNode &node, const RooAbsBinning &binning);
 
 #endif