From 3e06a9cfb80453b7f701d6908bf61243cc737b0e Mon Sep 17 00:00:00 2001
From: Jonas Rembser <jonas.rembser@cern.ch>
Date: Thu, 27 Jun 2024 13:17:35 +0200
Subject: [PATCH] [RF] Disable RooFit::Evaluator in RooFuncWrapper NLL tests

Disable the `RooFit::Evaluator` in the RooFuncWrapper NLL tests, such
that the generated code is used for the nominal likelihood too, and not
just to generate the gradient.

Like this, we also validate the nomial likelihood value from the
generated code.
---
 roofit/roofitcore/inc/RooFuncWrapper.h        | 5 ++++-
 roofit/roofitcore/src/RooFuncWrapper.cxx      | 6 +++---
 roofit/roofitcore/test/testRooFuncWrapper.cxx | 7 +++++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/roofit/roofitcore/inc/RooFuncWrapper.h b/roofit/roofitcore/inc/RooFuncWrapper.h
index c78badaa0c03e..bb4ca9480115c 100644
--- a/roofit/roofitcore/inc/RooFuncWrapper.h
+++ b/roofit/roofitcore/inc/RooFuncWrapper.h
@@ -33,7 +33,7 @@ namespace Experimental {
 class RooFuncWrapper final : public RooAbsReal {
 public:
    RooFuncWrapper(const char *name, const char *title, RooAbsReal &obj, const RooAbsData *data = nullptr,
-                  RooSimultaneous const *simPdf = nullptr, bool useEvaluator=false);
+                  RooSimultaneous const *simPdf = nullptr, bool useEvaluator = false);
 
    RooFuncWrapper(const RooFuncWrapper &other, const char *name = nullptr);
 
@@ -59,6 +59,8 @@ class RooFuncWrapper final : public RooAbsReal {
 
    void createGradient();
 
+   void disableEvaluator() { _useEvaluator = false; }
+
 protected:
    double evaluate() const override;
 
@@ -89,6 +91,7 @@ class RooFuncWrapper final : public RooAbsReal {
    Func _func;
    Grad _grad;
    bool _hasGradient = false;
+   bool _useEvaluator = false;
    mutable std::vector<double> _gradientVarBuffer;
    std::vector<double> _observables;
    std::map<RooFit::Detail::DataKey, ObsInfo> _obsInfos;
diff --git a/roofit/roofitcore/src/RooFuncWrapper.cxx b/roofit/roofitcore/src/RooFuncWrapper.cxx
index 6d6ddbdb9f02b..3bd59e9db962f 100644
--- a/roofit/roofitcore/src/RooFuncWrapper.cxx
+++ b/roofit/roofitcore/src/RooFuncWrapper.cxx
@@ -32,9 +32,9 @@ namespace Experimental {
 
 RooFuncWrapper::RooFuncWrapper(const char *name, const char *title, RooAbsReal &obj, const RooAbsData *data,
                                RooSimultaneous const *simPdf, bool useEvaluator)
-   : RooAbsReal{name, title}, _params{"!params", "List of parameters", this}
+   : RooAbsReal{name, title}, _params{"!params", "List of parameters", this}, _useEvaluator{useEvaluator}
 {
-   if (useEvaluator) {
+   if (_useEvaluator) {
       _absReal = std::make_unique<RooEvaluatorWrapper>(obj, const_cast<RooAbsData *>(data), false, "", simPdf, false);
    }
 
@@ -194,7 +194,7 @@ void RooFuncWrapper::updateGradientVarBuffer() const
 
 double RooFuncWrapper::evaluate() const
 {
-   if (_absReal)
+   if (_useEvaluator)
       return _absReal->getVal();
    updateGradientVarBuffer();
 
diff --git a/roofit/roofitcore/test/testRooFuncWrapper.cxx b/roofit/roofitcore/test/testRooFuncWrapper.cxx
index dd58452602ae8..24102d1803244 100644
--- a/roofit/roofitcore/test/testRooFuncWrapper.cxx
+++ b/roofit/roofitcore/test/testRooFuncWrapper.cxx
@@ -239,6 +239,10 @@ TEST_P(FactoryTest, NLLFit)
    std::unique_ptr<RooAbsReal> nllRef = _params._createNLL(model, *data, ws, RooFit::EvalBackend::Cpu());
    std::unique_ptr<RooAbsReal> nllFunc = _params._createNLL(model, *data, ws, RooFit::EvalBackend::Codegen());
 
+   // We don't use the RooFit::Evaluator for the nominal likelihood. Like this,
+   // we make sure to validate also the NLL values of the generated code.
+   static_cast<RooFit::Experimental::RooFuncWrapper &>(*nllFunc).disableEvaluator();
+
    double tol = _params._fitResultTolerance;
 
    EXPECT_NEAR(nllRef->getVal(observables), nllFunc->getVal(), tol);
@@ -585,8 +589,7 @@ FactoryTestParams param13{"RooFormulaVar",
                           /*randomizeParameters=*/false};
 
 // Test for the uniform pdf. Since it doesn't depend on any parameters, we need
-// to add it with some other model like a Gaussian to get a meaningful model to
-// fit.
+// to add it to some other model like a Gaussian to get a meaningful fit model.
 FactoryTestParams param14{"Uniform",
                           [](RooWorkspace &ws) {
                              ws.factory("Gaussian::sig(x[0, 10], mean[5, -10, 10], sigma1[0.50, .01, 10])");