diff --git a/roofit/codegen/src/CodegenImpl.cxx b/roofit/codegen/src/CodegenImpl.cxx index c0186c5e9bc0c..3199a5311159a 100644 --- a/roofit/codegen/src/CodegenImpl.cxx +++ b/roofit/codegen/src/CodegenImpl.cxx @@ -111,18 +111,33 @@ void rooHistTranslateImpl(RooAbsArg const &arg, CodegenContext &ctx, int intOrde ctx.addResult(&arg, "(" + weightArr + ")[" + offset + "]"); } -std::string realSumPdfTranslateImpl(CodegenContext &ctx, RooAbsArg const &arg, RooArgList const &funcList, - RooArgList const &coefList, bool normalize) +std::string +realSumPdfTranslateImpl(CodegenContext &ctx, RooArgList const &funcList, RooArgList const &coefList, bool normalize) { bool noLastCoeff = funcList.size() != coefList.size(); + // Decide the scope from the inputs rather than from the RooRealSumPdf + // itself: this function is also used for the integral of the sum, whose + // inputs are the integrals of the components. Those usually don't depend + // on the data even though the sum does, and then the code must not end up + // inside the event loop, because the integral is also the expected number + // of events that the extended likelihood uses after the loop. + bool scopeIndependent = true; + for (RooAbsArg const *func : funcList) { + scopeIndependent = scopeIndependent && ctx.isScopeIndependent(func); + } + for (RooAbsArg const *coef : coefList) { + scopeIndependent = scopeIndependent && ctx.isScopeIndependent(coef); + } + auto addCode = [&](std::string const &code) { ctx.addToCodeBody(code, scopeIndependent); }; + std::string const &funcName = ctx.buildArg(funcList); std::string const &coeffName = ctx.buildArg(coefList); std::string const &coeffSize = std::to_string(coefList.size()); std::string sum = ctx.getTmpVarName(); std::string coeffSum = ctx.getTmpVarName(); - ctx.addToCodeBody(&arg, "double " + sum + " = 0;\ndouble " + coeffSum + "= 0;\n"); + addCode("double " + sum + " = 0;\ndouble " + coeffSum + "= 0;\n"); std::string iterator = "i_" + ctx.getTmpVarName(); std::string subscriptExpr = "[" + iterator + "]"; @@ -137,7 +152,7 @@ std::string realSumPdfTranslateImpl(CodegenContext &ctx, RooAbsArg const &arg, R } else if (normalize) { code += sum + " /= " + coeffSum + ";\n"; } - ctx.addToCodeBody(&arg, code); + addCode(code); return sum; } @@ -249,7 +264,7 @@ void codegenImpl(RooAbsArg &arg, CodegenContext &ctx) void codegenImpl(RooAddPdf &arg, CodegenContext &ctx) { - ctx.addResult(&arg, realSumPdfTranslateImpl(ctx, arg, arg.pdfList(), arg.coefList(), true)); + ctx.addResult(&arg, realSumPdfTranslateImpl(ctx, arg.pdfList(), arg.coefList(), true)); } void codegenImpl(RooMultiVarGaussian &arg, CodegenContext &ctx) @@ -832,12 +847,12 @@ void codegenImpl(RooRealIntegral &arg, CodegenContext &ctx) void codegenImpl(RooRealSumFunc &arg, CodegenContext &ctx) { - ctx.addResult(&arg, realSumPdfTranslateImpl(ctx, arg, arg.funcList(), arg.coefList(), false)); + ctx.addResult(&arg, realSumPdfTranslateImpl(ctx, arg.funcList(), arg.coefList(), false)); } void codegenImpl(RooRealSumPdf &arg, CodegenContext &ctx) { - ctx.addResult(&arg, realSumPdfTranslateImpl(ctx, arg, arg.funcList(), arg.coefList(), false)); + ctx.addResult(&arg, realSumPdfTranslateImpl(ctx, arg.funcList(), arg.coefList(), false)); } void codegenImpl(RooRealVar &arg, CodegenContext &ctx) @@ -1100,7 +1115,7 @@ std::string codegenIntegralImpl(RooPolynomial &arg, int, const char *rangeName, std::string codegenIntegralImpl(RooRealSumPdf &arg, int code, const char *rangeName, CodegenContext &ctx) { // Re-use translate, since integration is linear. - return realSumPdfTranslateImpl(ctx, arg, arg.funcIntListFromCache(code, rangeName), arg.coefList(), false); + return realSumPdfTranslateImpl(ctx, arg.funcIntListFromCache(code, rangeName), arg.coefList(), false); } std::string codegenIntegralImpl(RooUniform &arg, int code, const char *rangeName, CodegenContext &) diff --git a/roofit/roofitcore/inc/RooFit/CodegenContext.h b/roofit/roofitcore/inc/RooFit/CodegenContext.h index 18e70482c5453..1ef74da53de5c 100644 --- a/roofit/roofitcore/inc/RooFit/CodegenContext.h +++ b/roofit/roofitcore/inc/RooFit/CodegenContext.h @@ -65,6 +65,8 @@ class CodegenContext { void addToCodeBody(std::string const &in, bool isScopeIndep = false); + bool isScopeIndependent(RooAbsArg const *in) const; + /// @brief Build the code to call the function with name `funcname`, passing some arguments. /// The arguments can either be doubles or some RooFit arguments whose /// results will be looked up in the context. @@ -130,8 +132,6 @@ class CodegenContext { template std::string buildArgSpanImpl(std::span arr); - bool isScopeIndependent(RooAbsArg const *in) const; - void endLoop(LoopScope const &scope); void addResult(TNamed const *key, std::string const &value); diff --git a/roofit/roofitcore/test/testRooFuncWrapper.cxx b/roofit/roofitcore/test/testRooFuncWrapper.cxx index 97d1b8d416199..801b6c7e68aff 100644 --- a/roofit/roofitcore/test/testRooFuncWrapper.cxx +++ b/roofit/roofitcore/test/testRooFuncWrapper.cxx @@ -650,6 +650,15 @@ auto testValues = testing::Values( "RooRealSumPdf1", {"Gaussian::gx(x[-10,10],m[0],1.0)", "Chebychev::ch(x,{0.1,0.2,-0.3})", "RealSumPdf::model({gx, ch}, {f[0,1]})"}, 6e-3, false), + // Extended RooRealSumPdf (one coefficient per function, like in HistFactory + // models). The expected number of events is the integral of the sum, which + // the NLL reads after the event loop. Here, the same integral is also the + // normalization of the pdf inside the loop, so this checks that the + // generated integral code is placed outside the loop. + makeTestParams("RooRealSumPdfExtended", + {"Gaussian::gx(x[-10,10],m[0,-3,3],1.5)", "Exponential::ex(x,-0.1)", + "ASUM::model(n1[100,0,1000]*gx, n2[100,0,1000]*ex)"}, + 6e-3, true), // Express a Gaussian as a RooFormulaVar/expr and put in a RooWrapper pdf to // get test coverage for RooWrapperPdf. makeTestParams("RooWrapperPdf",