[RF][HS3] Sort variables exported by RooGenericPdf and FooFormulaVar

This makes the generated JSON more reproducible.

Author: guitargeek

roofit/hs3/src/JSONFactories_RooFitCore.cxx

@@ -17,14 +17,14 @@
 #include <RooBinWidthFunction.h>
 #include <RooCategory.h>
 #include <RooDataHist.h>
-#include <RooLegacyExpPoly.h>
 #include <RooExponential.h>
 #include <RooFit/Detail/JSONInterface.h>
 #include <RooFitHS3/JSONIO.h>
 #include <RooFormulaVar.h>
 #include <RooGenericPdf.h>
 #include <RooHistFunc.h>
 #include <RooHistPdf.h>
+#include <RooLegacyExpPoly.h>
 #include <RooLognormal.h>
 #include <RooMultiVarGaussian.h>
 #include <RooPoisson.h>
@@ -35,11 +35,11 @@
 #include <RooTFnBinding.h>
 #include <RooWorkspace.h>
 
-#include "JSONIOUtils.h"
-
 #include <TF1.h>
 #include <TH1.h>
 
+#include "JSONIOUtils.h"
+
 #include "static_execute.h"
 
 #include <algorithm>
@@ -519,14 +519,15 @@ class RooFormulaArgStreamer : public RooFit::JSONIO::Exporter {
       const RooArg_t *pdf = static_cast<const RooArg_t *>(func);
       elem["type"] << key();
       TString expression(pdf->expression());
+      std::vector<std::pair<RooAbsArg *, std::size_t>> paramsWithIndex;
+      paramsWithIndex.reserve(pdf->nParameters());
       for (size_t i = 0; i < pdf->nParameters(); ++i) {
-         RooAbsArg *par = pdf->getParameter(i);
-         std::stringstream ss_1;
-         ss_1 << "x[" << i << "]";
-         std::stringstream ss_2;
-         ss_2 << "@" << i << "";
-         expression.ReplaceAll(ss_1.str().c_str(), par->GetName());
-         expression.ReplaceAll(ss_2.str().c_str(), par->GetName());
+         paramsWithIndex.emplace_back(pdf->getParameter(i), i);
+      }
+      std::sort(paramsWithIndex.begin(), paramsWithIndex.end());
+      for (auto [par, idx] : paramsWithIndex) {
+         expression.ReplaceAll(("x[" + std::to_string(idx) + "]").c_str(), par->GetName());
+         expression.ReplaceAll(("@" + std::to_string(idx)).c_str(), par->GetName());
       }
       elem["expression"] << expression.Data();
       return true;

roofit/hs3/src/RooJSONFactoryWSTool.cxx

@@ -100,6 +100,16 @@ using RooFit::Detail::JSONTree;
 
 namespace {
 
+std::vector<std::string> valsToStringVec(JSONNode const &node)
+{
+   std::vector<std::string> out;
+   out.reserve(node.num_children());
+   for (JSONNode const &elem : node.children()) {
+      out.push_back(elem.val());
+   }
+   return out;
+}
+
 /**
  * @brief Check if the number of components in CombinedData matches the number of categories in the RooSimultaneous PDF.
  *
@@ -587,16 +597,13 @@ void importAnalysis(const JSONNode &rootnode, const JSONNode &analysisNode, cons
    auto *mc = static_cast<RooStats::ModelConfig *>(workspace.obj(mcname));
    mc->SetWS(workspace);
 
-   std::vector<std::string> nllDistNames;
    std::vector<std::string> nllDataNames;
 
    auto *nllNode = RooJSONFactoryWSTool::findNamedChild(likelihoodsNode, analysisNode["likelihood"].val());
    if (!nllNode) {
       throw std::runtime_error("likelihood node not found!");
    }
-   for (auto &nameNode : (*nllNode)["distributions"].children()) {
-      nllDistNames.push_back(nameNode.val());
-   }
+   std::vector<std::string> nllDistNames = valsToStringVec((*nllNode)["distributions"]);
    RooArgSet extConstraints;
    for (auto &nameNode : (*nllNode)["aux_distributions"].children()) {
       RooAbsArg *extConstraint = workspace.arg(nameNode.val());
@@ -699,18 +706,12 @@ void combinePdfs(const JSONNode &rootnode, RooWorkspace &ws)
       // parse the information
       std::string combinedName = info.key();
       std::string indexCatName = info["index_cat"].val();
-      std::vector<std::string> labels;
+      std::vector<std::string> labels = valsToStringVec(info["labels"]);
       std::vector<int> indices;
-      std::vector<std::string> pdfNames;
+      std::vector<std::string> pdfNames = valsToStringVec(info["distributions"]);
       for (auto &n : info["indices"].children()) {
          indices.push_back(n.val_int());
       }
-      for (auto &n : info["labels"].children()) {
-         labels.push_back(n.val());
-      }
-      for (auto &n : info["distributions"].children()) {
-         pdfNames.push_back(n.val());
-      }
 
       RooCategory indexCat{indexCatName.c_str(), indexCatName.c_str()};
       std::map<std::string, RooAbsPdf *> pdfMap;
@@ -739,14 +740,11 @@ void combineDatasets(const JSONNode &rootnode, std::vector<std::unique_ptr<RooAb
       // parse the information
       std::string combinedName = info.key();
       std::string indexCatName = info["index_cat"].val();
-      std::vector<std::string> labels;
+      std::vector<std::string> labels = valsToStringVec(info["labels"]);
       std::vector<int> indices;
       for (auto &n : info["indices"].children()) {
          indices.push_back(n.val_int());
       }
-      for (auto &n : info["labels"].children()) {
-         labels.push_back(n.val());
-      }
       if (indices.size() != labels.size()) {
          RooJSONFactoryWSTool::error("mismatch in number of indices and labels!");
       }

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

@@ -37,10 +37,6 @@ class JSONNode {
          virtual bool equal(const Impl &other) const = 0;
       };
 
-   private:
-      std::unique_ptr<Impl> it;
-
-   public:
       child_iterator_t(std::unique_ptr<Impl> impl) : it(std::move(impl)) {}
       child_iterator_t(const child_iterator_t &other) : it(std::move(other.it->clone())) {}
 
@@ -65,6 +61,9 @@ class JSONNode {
       {
          return lhs.it->equal(*rhs.it);
       }
+
+   private:
+      std::unique_ptr<Impl> it;
    };
 
    using child_iterator = child_iterator_t<JSONNode>;