[RF] Don't disable dirty flag propagation during numeric integrals

documentation/doxygen/Doxyfile

@@ -1080,7 +1080,6 @@ EXCLUDE_PATTERNS       = */G__* \
                          */gui/cefdisplay/* \
                          */gui/qt6webdisplay/* \
                          */tutorials/visualisation/webgui/qtweb/* \
-                         */tutorials/roofit/roostats/rs401* \
                          */math/mathcore/src/CDT*
 
 # The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names

roofit/roofitcore/res/RooFitImplHelpers.h

@@ -57,6 +57,8 @@ class ChangeOperModeRAII {
 
    ChangeOperModeRAII(ChangeOperModeRAII const &other) = delete;
    ChangeOperModeRAII &operator=(ChangeOperModeRAII const &other) = delete;
+   ChangeOperModeRAII(ChangeOperModeRAII &&other) = delete;
+   ChangeOperModeRAII &operator=(ChangeOperModeRAII &&other) = delete;
 
    ~ChangeOperModeRAII() { _arg->setOperMode(_oldOpMode, /*recurse=*/false); }
 

roofit/roofitcore/src/RooAbsReal.cxx

@@ -4217,6 +4217,8 @@ void RooAbsReal::doEval(RooFit::EvalContext & ctx) const
     auto serverValues = ctx.at(server);
     if(serverValues.empty()) continue;
 
+    if(!server->isValueServer(*this)) continue;
+
     // maybe we are still missing inhibit dirty here
     auto oldOperMode = server->operMode();
     // See note at the bottom of this function to learn why we can only set

roofit/roofitcore/src/RooRealIntegral.cxx

@@ -43,6 +43,7 @@ integration is performed in the various implementations of the RooAbsIntegrator
 #include <RooRealBinding.h>
 #include <RooSuperCategory.h>
 #include <RooTrace.h>
+#include <RooFitImplHelpers.h>
 
 #include <iostream>
 #include <memory>
@@ -377,7 +378,7 @@ RooRealIntegral::RooRealIntegral(const char *name, const char *title,
   }
 
   if (!_facList.empty()) {
-    oocxcoutI(&function,Integration) << function.GetName() << ": Factorizing obserables are " << _facList << std::endl ;
+    oocxcoutI(&function,Integration) << function.GetName() << ": Factorizing observables are " << _facList << std::endl ;
   }
 
 
@@ -814,28 +815,50 @@ double RooRealIntegral::evaluate() const
 
   case Hybrid:
     {
-        // Find any function dependents that are AClean
-        // and switch them temporarily to ADirty
-        bool origState = inhibitDirty() ;
-        setDirtyInhibit(true) ;
-
         // try to initialize our numerical integration engine
         if(!(_valid= initNumIntegrator())) {
           coutE(Integration) << ClassName() << "::" << GetName()
                              << ":evaluate: cannot initialize numerical integrator" << std::endl;
           return 0;
         }
 
+        // Find any function dependents that are "AClean" and switch them temporarily to "Auto".
+        // We do this by compute graph traversal and RAII objects on the heap,
+        // which seems quite expensive, but is not as bad as it looks because:
+        //   1. The sub-graphs representing numerically-integrated functions
+        //      are usually small
+        //   2. The numerical integration itself dominates the runtime of the
+        //      evaluation.
+        //   3. The operMode is only "AClean" if we use the constant term
+        //      optimization of the legacy test statistics.
+        //   4. Once the legacy test statistics are deprecated and removed,
+        //      this code block can go away (TODO when that happens).
+        // Note: in the past, the "AClean" states were changed with a global
+        // setDirtyInhibit(true) before evaluating the numeric integral. While
+        // this avoids the bookkeeping overhead, it actually changes the oper
+        // mode of all nodes to "ADirty" and not to "Auto", resulting in
+        // significant performance loss in case the target function benefits
+        // from caching subgraph results (e.g. for nested numeric integrals).
+        RooArgList serverList;
+        _function->treeNodeServerList(&serverList, nullptr, true, true, false, true);
+        std::list<ChangeOperModeRAII> operModeRAII;
+
+        for (auto *arg : serverList) {
+           arg->syncCache();
+           if (arg->operMode() == RooAbsArg::AClean) {
+              operModeRAII.emplace_back(arg, RooAbsArg::Auto);
+           }
+        }
+
         // Save current integral dependent values
         _saveInt.assign(_intList) ;
         _saveSum.assign(_sumList) ;
 
         // Evaluate sum/integral
         retVal = sum() ;
 
-
         // This must happen BEFORE restoring dependents, otherwise no dirty state propagation in restore step
-        setDirtyInhibit(origState) ;
+        operModeRAII.clear();
 
         // Restore integral dependent values
         _intList.assign(_saveInt) ;
@@ -863,9 +886,7 @@ double RooRealIntegral::evaluate() const
       // integrated over later.
       assert(servers().size() == _facList.size() + 1);
 
-      //setDirtyInhibit(true) ;
       retVal= _function->getVal(actualFuncNormSet());
-      //setDirtyInhibit(false) ;
       break ;
     }
   }