[RF] Don't normalize over non-dependents when compiling for norm. set

Don't normalize pdfs over non-dependents in
`RooAbsPdf::compileForNormSet()`.

This fixes use cases where pdfs that don't depend on any observables are
used for their unnormalized `RooAbsPdf::evaluate()` shape as functions.

Even thought technically not allowed because evaluating pdfs without a
normalization set is undefined, this pattern is used a lot in the wild,
so we need to support it also in the new vectorizing evaluation backend.

A unit test that covers the original user-reported problem is also
added.

Author: guitargeek

roofit/roofitcore/src/RooAbsPdf.cxx

@@ -2805,10 +2805,19 @@ RooAbsPdf::compileForNormSet(RooArgSet const &normSet, RooFit::Detail::CompileCo
    if (normSet.empty() || selfNormalized()) {
       return RooAbsReal::compileForNormSet(normSet, ctx);
    }
+
+   // Just like in syncNormalization() for the RooAbsPdf::getValV() code path,
+   // we're not integrating over any variables that the pdf doesn't depend on.
+   RooArgSet depList;
+   getObservables(&normSet, depList);
+   if (depList.empty()) {
+      return RooAbsReal::compileForNormSet(normSet, ctx);
+   }
+
    std::unique_ptr<RooAbsPdf> pdfClone(static_cast<RooAbsPdf *>(this->Clone()));
-   ctx.compileServers(*pdfClone, normSet);
+   ctx.compileServers(*pdfClone, depList);
 
-   auto newArg = std::make_unique<RooFit::Detail::RooNormalizedPdf>(*pdfClone, normSet);
+   auto newArg = std::make_unique<RooFit::Detail::RooNormalizedPdf>(*pdfClone, depList);
 
    // The direct servers are this pdf and the normalization integral, which
    // don't need to be compiled further.

roofit/roofitcore/test/CMakeLists.txt

@@ -18,7 +18,7 @@ ROOT_ADD_GTEST(testRooDataHist testRooDataHist.cxx LIBRARIES RooFitCore
 ROOT_ADD_GTEST(testRooBinSamplingPdf testRooBinSamplingPdf.cxx LIBRARIES RooFitCore)
 ROOT_ADD_GTEST(testRooWrapperPdf testRooWrapperPdf.cxx LIBRARIES Gpad RooFitCore)
 ROOT_ADD_GTEST(testGenericPdf testGenericPdf.cxx LIBRARIES RooFitCore)
-ROOT_ADD_GTEST(testRooAbsPdf testRooAbsPdf.cxx LIBRARIES RooFitCore)
+ROOT_ADD_GTEST(testRooAbsPdf testRooAbsPdf.cxx LIBRARIES RooFitCore RooFit)
 ROOT_ADD_GTEST(testRooAbsCollection testRooAbsCollection.cxx LIBRARIES RooFitCore)
 
 ROOT_ADD_GTEST(testRooDataSet testRooDataSet.cxx LIBRARIES Tree RooFitCore

roofit/roofitcore/test/testRooAbsPdf.cxx

@@ -2,23 +2,26 @@
 // Authors: Stephan Hageboeck, CERN 04/2020
 //          Jonas Rembser, CERN 04/2021
 
-#include <RooAddition.h>
 #include <RooAddPdf.h>
+#include <RooAddition.h>
 #include <RooCategory.h>
 #include <RooConstVar.h>
 #include <RooDataHist.h>
 #include <RooDataSet.h>
 #include <RooFitResult.h>
 #include <RooFormulaVar.h>
+#include <RooGaussian.h>
 #include <RooGenericPdf.h>
 #include <RooHelpers.h>
+#include <RooParametricStepFunction.h>
 #include <RooProdPdf.h>
 #include <RooProduct.h>
+#include <RooRandom.h>
 #include <RooRealVar.h>
 #include <RooSimultaneous.h>
 #include <RooWorkspace.h>
-#include <RooRandom.h>
 
+#include <TArrayD.h>
 #include <TClass.h>
 #include <TRandom.h>
 
@@ -375,6 +378,42 @@ TEST_P(FitTest, ProblemsWith2DSimultaneousFit)
    simPdf.fitTo(*data, PrintLevel(-1), _evalBackend);
 }
 
+// This test covers a usecase by an ATLAS collaborator. The unnormalized shape
+// of a RooFit pdf is used as a function inside a RooFormulaVar, which is used
+// for the bins of a RooParametricStep function. This case is potentially
+// fragile, because it requires that the top-level normalization set is ignored
+// for the inner pdfs that don't depend on the observable, as normalizing over
+// a non-dependent is a corner case where the variable should be dropped.
+TEST_P(FitTest, PdfAsFunctionInFormulaVar)
+{
+   using namespace RooFit;
+
+   RooRealVar x{"x", "x", 0., 1.};
+
+   const double arg = std::sqrt(-8 * std::log(0.5));
+   RooGaussian gauss1{"gauss1", "", 10 - arg, 10, 2};
+
+   RooFormulaVar func1{"func1", "x[0]", {gauss1}};
+
+   // The parametric step function doesn't make any attempt at self
+   // normalization, so the pdf value in the first bin is just the value of the
+   // unnormalized Gaussian. And the first bin is the almost the whole domain.
+   TArrayD limits(3);
+   limits[0] = 0.;
+   limits[1] = 1 - 1e-9;
+   limits[2] = 1.;
+   RooParametricStepFunction pdf("pdf", "pdf", x, {func1}, limits, limits.size() - 1);
+
+   int nEvents = 10000;
+   std::unique_ptr<RooAbsData> data{pdf.generateBinned(x, nEvents)};
+
+   std::unique_ptr<RooAbsReal> nll{pdf.createNLL(*data, _evalBackend)};
+
+   // The test is designed to have an analytical reference value
+   double ref = nEvents * -std::log(0.5);
+   EXPECT_FLOAT_EQ(nll->getVal(), ref);
+}
+
 // Verifies that a server pdf gets correctly reevaluated when the normalization
 // set is changed.
 TEST(RooAbsPdf, NormSetChange)