[RF][HS3] Add Workspace => JSON => Workspace => JSON check to tests

Make sure that objects in a list are always exported to JOSN
alphabeticall, so the JSON 1 vs. JSON 2 comparision doesn't fail.

Author: guitargeek

roofit/hs3/inc/RooFitHS3/RooJSONFactoryWSTool.h

@@ -194,6 +194,20 @@ class RooJSONFactoryWSTool {
 
    void exportObject(RooAbsArg const &func, std::set<std::string> &exportedObjectNames);
 
+   // To export multiple objects sorted alphabetically
+   template <class T>
+   void exportObjects(T const &args, std::set<std::string> &exportedObjectNames)
+   {
+      RooArgSet argSet;
+      for (RooAbsArg const *arg : args) {
+         argSet.add(*arg);
+      }
+      argSet.sort();
+      for (RooAbsArg *arg : argSet) {
+         exportObject(*arg, exportedObjectNames);
+      }
+   }
+
    void exportData(RooAbsData const &data);
    RooJSONFactoryWSTool::CombinedData exportCombinedData(RooAbsData const &data);
 

roofit/hs3/src/RooJSONFactoryWSTool.cxx

@@ -606,8 +606,7 @@ void importAnalysis(const JSONNode &rootnode, const JSONNode &analysisNode, cons
    std::vector<std::string> nllDistNames = valsToStringVec((*nllNode)["distributions"]);
    RooArgSet extConstraints;
    for (auto &nameNode : (*nllNode)["aux_distributions"].children()) {
-      RooAbsArg *extConstraint = workspace.arg(nameNode.val());
-      if (extConstraint) {
+      if (RooAbsArg *extConstraint = workspace.arg(nameNode.val())) {
          extConstraints.add(*extConstraint);
       }
    }
@@ -989,9 +988,7 @@ void RooJSONFactoryWSTool::exportObject(RooAbsArg const &func, std::set<std::str
    if (auto simPdf = dynamic_cast<RooSimultaneous const *>(&func)) {
       // RooSimultaneous is not used in the HS3 standard, we only export the
       // dependents and some ROOT internal information.
-      for (RooAbsArg *s : func.servers()) {
-         this->exportObject(*s, exportedObjectNames);
-      }
+      exportObjects(func.servers(), exportedObjectNames);
 
       std::vector<std::string> channelNames;
       for (auto const &item : simPdf->indexCat()) {
@@ -1040,13 +1037,9 @@ void RooJSONFactoryWSTool::exportObject(RooAbsArg const &func, std::set<std::str
             continue;
          }
          if (exp->autoExportDependants()) {
-            for (RooAbsArg *s : func.servers()) {
-               this->exportObject(*s, exportedObjectNames);
-            }
+            exportObjects(func.servers(), exportedObjectNames);
          } else {
-            for (RooAbsArg const *s : _serversToExport) {
-               this->exportObject(*s, exportedObjectNames);
-            }
+            exportObjects(_serversToExport, exportedObjectNames);
          }
          return;
       }
@@ -1758,9 +1751,7 @@ void RooJSONFactoryWSTool::exportAllObjects(JSONNode &n)
    }
    sortByName(allpdfs);
    std::set<std::string> exportedObjectNames;
-   for (RooAbsPdf *p : allpdfs) {
-      this->exportObject(*p, exportedObjectNames);
-   }
+   exportObjects(allpdfs, exportedObjectNames);
 
    // export attributes of all objects
    for (RooAbsArg *arg : _workspace.components()) {

roofit/hs3/test/testRooFitHS3.cxx

@@ -29,7 +29,7 @@
 namespace {
 
 // If the JSON files should be written out for debugging purpose.
-const bool writeJsonFiles = true;
+const bool writeJsonFiles = false;
 
 // Validate the JSON IO for a given RooAbsReal in a RooWorkspace. The workspace
 // will be written out and read back, and then the values of the old and new
@@ -39,10 +39,33 @@ int validate(RooWorkspace &ws1, std::string const &argName, bool exact = true)
 {
    RooWorkspace ws2;
 
+   const std::string json1 = RooJSONFactoryWSTool{ws1}.exportJSONtoString();
+   RooJSONFactoryWSTool{ws2}.importJSONfromString(json1);
+
+   // Export the re-imported workspace back to JSON, and compare the first JSON
+   // with the second one. They should be identical.
+   const std::string json2 = RooJSONFactoryWSTool{ws2}.exportJSONtoString();
+   EXPECT_EQ(json2, json1) << argName;
+
    if (writeJsonFiles) {
-      RooJSONFactoryWSTool{ws1}.exportJSON(argName + ".json");
+      RooJSONFactoryWSTool{ws1}.exportJSON(argName + "_1.json");
+      RooJSONFactoryWSTool{ws2}.exportJSON(argName + "_2.json");
+   }
+
+   // It would be nice to do a similar closure check for the original and for
+   // the re-imported workspace. However, there is no way to compare workspaces
+   // for equality. But we can still check that the objects in the workspace
+   // have at least the same name.
+   RooArgSet comps1 = ws1.components();
+   RooArgSet comps2 = ws2.components();
+   EXPECT_EQ(comps2.size(), comps1.size());
+
+   comps1.sort();
+   comps2.sort();
+
+   for (std::size_t i = 0; i < comps1.size(); ++i) {
+      EXPECT_STREQ(comps1[i]->GetName(), comps2[i]->GetName());
    }
-   RooJSONFactoryWSTool{ws2}.importJSONfromString(RooJSONFactoryWSTool{ws1}.exportJSONtoString());
 
    RooRealVar &x1 = *ws1.var("x");
    RooRealVar &x2 = *ws2.var("x");