[RF] Handle nullptr or empty norm set ambiguity in RooAbsReal::getVal

roofit/roofitcore/inc/RooAbsReal.h

@@ -89,6 +89,14 @@ class RooAbsReal : public RooAbsArg {
   /// These are integrated over their current ranges to compute the normalisation constant,
   /// and the unnormalised result is divided by this value.
   inline Double_t getVal(const RooArgSet* normalisationSet = nullptr) const {
+    // Sometimes, the calling code uses an empty RooArgSet to request evaluation
+    // without normalization set instead of following the `nullptr` convention.
+    // To remove this ambiguity which might not always be correctly handled in
+    // downstream code, we set `normalisationSet` to nullptr if it is pointing
+    // to an empty set.
+    if(normalisationSet && normalisationSet->empty()) {
+      normalisationSet = nullptr;
+    }
 #ifdef ROOFIT_CHECK_CACHED_VALUES
     return _DEBUG_getVal(normalisationSet);
 #else
@@ -103,7 +111,13 @@ class RooAbsReal : public RooAbsArg {
   }
 
   /// Like getVal(const RooArgSet*), but always requires an argument for normalisation.
-  inline  Double_t getVal(const RooArgSet& normalisationSet) const { return _fast ? _value : getValV(&normalisationSet) ; }
+  inline  Double_t getVal(const RooArgSet& normalisationSet) const {
+    // Sometimes, the calling code uses an empty RooArgSet to request evaluation
+    // without normalization set instead of following the `nullptr` convention.
+    // To remove this ambiguity which might not always be correctly handled in
+    // downstream code, we set `normalisationSet` to nullptr if it is an empty set.
+    return _fast ? _value : getValV(normalisationSet.empty() ? nullptr : &normalisationSet) ;
+  }
 
   virtual Double_t getValV(const RooArgSet* normalisationSet = nullptr) const ;
 

roofit/roofitcore/test/CMakeLists.txt

@@ -37,4 +37,5 @@ if(NOT MSVC OR win_broken_tests)
 endif()
 ROOT_ADD_GTEST(testRooProductPdf testRooProductPdf.cxx LIBRARIES RooFitCore)
 ROOT_ADD_GTEST(testNaNPacker testNaNPacker.cxx LIBRARIES RooFitCore)
+ROOT_ADD_GTEST(testRooSimultaneous testRooSimultaneous.cxx LIBRARIES RooFitCore RooFit)
 

roofit/roofitcore/test/testRooSimultaneous.cxx

@@ -0,0 +1,49 @@
+// Tests for the RooSimultaneous
+// Authors: Jonas Rembser, CERN  06/2021
+
+#include "RooAddPdf.h"
+#include "RooConstVar.h"
+#include "RooCategory.h"
+#include "RooDataSet.h"
+#include "RooGaussian.h"
+#include "RooRealVar.h"
+#include "RooSimultaneous.h"
+#include "RooProdPdf.h"
+
+#include "gtest/gtest.h"
+
+#include <memory>
+
+/// GitHub issue #8307.
+/// A likelihood with a model wrapped in a RooSimultaneous ith one category
+/// should give the same results as the likelihood with the model directly.
+TEST(RooSimultaneous, ImportFromTreeWithCut)
+{
+   using namespace RooFit;
+
+   RooRealVar x("x", "x", 0, 10);
+   RooRealVar mean("mean", "mean", 1., 0, 10);
+   RooRealVar width("width", "width", 1, 0.1, 10);
+   RooRealVar nsig("nsig", "nsig", 500, 100, 1000);
+
+   RooGaussian gauss1("gauss1", "gauss1", x, mean, width);
+   RooGaussian fconstraint("fconstraint", "fconstraint", mean, RooConst(2.0), RooConst(0.2));
+
+   RooAddPdf model("model", "model", RooArgList(gauss1), RooArgList(nsig));
+   RooProdPdf modelConstrained("modelConstrained", "modelConstrained", RooArgSet(model, fconstraint));
+
+   RooCategory cat("cat", "cat");
+   cat.defineType("physics");
+
+   RooSimultaneous modelSim("modelSim", "modelSim", RooArgList{modelConstrained}, cat);
+
+   std::unique_ptr<RooDataSet> data{model.generate(x)};
+   RooDataSet combData("combData", "combData", x, Index(cat), Import("physics", *data));
+
+   RooArgSet constraints{fconstraint};
+
+   std::unique_ptr<RooAbsReal> nllDirect{modelConstrained.createNLL(combData, Constrain(constraints))};
+   std::unique_ptr<RooAbsReal> nllSimWrapped{modelSim.createNLL(combData, Constrain(constraints))};
+
+   EXPECT_FLOAT_EQ(nllDirect->getVal(), nllSimWrapped->getVal());
+}