diff --git a/roofit/roofitcore/inc/RooAbsReal.h b/roofit/roofitcore/inc/RooAbsReal.h index 465407e5325b2..53eb63fea1119 100644 --- a/roofit/roofitcore/inc/RooAbsReal.h +++ b/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 ; diff --git a/roofit/roofitcore/test/CMakeLists.txt b/roofit/roofitcore/test/CMakeLists.txt index 7b55450808091..85e69a85a6afc 100644 --- a/roofit/roofitcore/test/CMakeLists.txt +++ b/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) diff --git a/roofit/roofitcore/test/testRooSimultaneous.cxx b/roofit/roofitcore/test/testRooSimultaneous.cxx new file mode 100644 index 0000000000000..1d9afb7b9297c --- /dev/null +++ b/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 + +/// 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 data{model.generate(x)}; + RooDataSet combData("combData", "combData", x, Index(cat), Import("physics", *data)); + + RooArgSet constraints{fconstraint}; + + std::unique_ptr nllDirect{modelConstrained.createNLL(combData, Constrain(constraints))}; + std::unique_ptr nllSimWrapped{modelSim.createNLL(combData, Constrain(constraints))}; + + EXPECT_FLOAT_EQ(nllDirect->getVal(), nllSimWrapped->getVal()); +}