[RF] Move normalizeWithNaNPacking to RooFit::Detail namespace

This means that other RooFit classes can use this function in their
implementation without having to rely on friend classes.

Author: guitargeek

roofit/batchcompute/res/RooNaNPacker.h

@@ -13,7 +13,7 @@
 #ifndef ROOFIT_BATCHCOMPUTE_ROONANPACKER_H
 #define ROOFIT_BATCHCOMPUTE_ROONANPACKER_H
 
-#include <RooBatchComputeTypes.h>
+#include "./RooBatchComputeTypes.h"
 
 #include <TError.h>
 

roofit/roofitcore/inc/RooAbsPdf.h

@@ -296,10 +296,6 @@ class RooAbsPdf : public RooAbsReal {
     return RooFit::getUniqueId(normSet).value() == _normSetId;
   }
 
-  friend class NaNPackingNormalizer;
-
-  double normalizeWithNaNPacking(double rawVal, double normVal) const;
-
   RooPlot *plotOn(RooPlot *frame, PlotOpt o) const override;
 
   friend class RooMCStudy ;

roofit/roofitcore/inc/RooFit/Detail/RooNormalizedPdf.h

@@ -82,10 +82,7 @@ class RooNormalizedPdf : public RooAbsPdf {
       // still need it to support printing of the object.
       return getValV(nullptr);
    }
-   double getValV(const RooArgSet * /*normSet*/) const override
-   {
-      return normalizeWithNaNPacking(_pdf->getVal(), _normIntegral->getVal());
-   };
+   double getValV(const RooArgSet * normSet) const override;
 
 private:
    RooTemplateProxy<RooAbsPdf> _pdf;

roofit/roofitcore/res/RooFitImplHelpers.h

@@ -11,9 +11,14 @@
 #ifndef RooFit_RooFitImplHelpers_h
 #define RooFit_RooFitImplHelpers_h
 
-#include <RooMsgService.h>
 #include <RooAbsArg.h>
+#include <RooAbsPdf.h>
 #include <RooAbsReal.h>
+#include <RooMsgService.h>
+
+#include "../../batchcompute/res/RooNaNPacker.h"
+
+#include <TMath.h>
 
 #include <sstream>
 #include <string>
@@ -104,6 +109,33 @@ void replaceAll(std::string &inOut, std::string_view what, std::string_view with
 
 std::string makeSliceCutString(RooArgSet const &sliceDataSet);
 
+// Inlined because this is called inside RooAbsPdf::getValV(), and therefore
+// performance critical.
+inline double normalizeWithNaNPacking(RooAbsPdf const &pdf, double rawVal, double normVal)
+{
+
+   if (normVal < 0. || (normVal == 0. && rawVal != 0)) {
+      // Unreasonable normalisations. A zero integral can be tolerated if the function vanishes, though.
+      const std::string msg = "p.d.f normalization integral is zero or negative: " + std::to_string(normVal);
+      pdf.logEvalError(msg.c_str());
+      return RooNaNPacker::packFloatIntoNaN(-normVal + (rawVal < 0. ? -rawVal : 0.));
+   }
+
+   if (rawVal < 0.) {
+      std::stringstream ss;
+      ss << "p.d.f value is less than zero (" << rawVal << "), trying to recover";
+      pdf.logEvalError(ss.str().c_str());
+      return RooNaNPacker::packFloatIntoNaN(-rawVal);
+   }
+
+   if (TMath::IsNaN(rawVal)) {
+      pdf.logEvalError("p.d.f value is Not-a-Number");
+      return rawVal;
+   }
+
+   return (rawVal == 0. && normVal == 0.) ? 0. : rawVal / normVal;
+}
+
 } // namespace RooFit::Detail
 
 double toDouble(const char *s);

roofit/roofitcore/src/RooAbsPdf.cxx

@@ -286,34 +286,6 @@ RooAbsPdf::~RooAbsPdf()
 }
 
 
-double RooAbsPdf::normalizeWithNaNPacking(double rawVal, double normVal) const {
-
-    if (normVal < 0. || (normVal == 0. && rawVal != 0)) {
-      //Unreasonable normalisations. A zero integral can be tolerated if the function vanishes, though.
-      const std::string msg = "p.d.f normalization integral is zero or negative: " + std::to_string(normVal);
-      logEvalError(msg.c_str());
-      clearValueAndShapeDirty();
-      return RooNaNPacker::packFloatIntoNaN(-normVal + (rawVal < 0. ? -rawVal : 0.));
-    }
-
-    if (rawVal < 0.) {
-       std::stringstream ss;
-       ss << "p.d.f value is less than zero (" << rawVal << "), trying to recover";
-       logEvalError(ss.str().c_str());
-       clearValueAndShapeDirty();
-       return RooNaNPacker::packFloatIntoNaN(-rawVal);
-    }
-
-    if (TMath::IsNaN(rawVal)) {
-      logEvalError("p.d.f value is Not-a-Number");
-      clearValueAndShapeDirty();
-      return rawVal;
-    }
-
-    return (rawVal == 0. && normVal == 0.) ? 0. : rawVal / normVal;
-}
-
-
 ////////////////////////////////////////////////////////////////////////////////
 /// Return current value, normalized by integrating over
 /// the observables in `nset`. If `nset` is 0, the unnormalized value
@@ -354,7 +326,7 @@ double RooAbsPdf::getValV(const RooArgSet* nset) const
     // Evaluate denominator
     const double normVal = _norm->getVal();
 
-    _value = normalizeWithNaNPacking(rawVal, normVal);
+    _value = RooFit::Detail::normalizeWithNaNPacking(*this, rawVal, normVal);
 
     clearValueAndShapeDirty();
   }

roofit/roofitcore/src/RooFFTConvPdf.cxx

@@ -123,6 +123,7 @@
 #include "RooGlobalFunc.h"
 #include "RooConstVar.h"
 #include "RooUniformBinning.h"
+#include "RooFitImplHelpers.h"
 
 #include "TClass.h"
 #include "TComplex.h"
@@ -205,15 +206,6 @@ void RooFFTConvPdf_doFFT(int n, double *input1, double *input2, double *output)
 
 #endif
 
-class NaNPackingNormalizer {
-public:
-   double getVal(RooAbsPdf const &pdf, double normVal)
-   {
-      double rawVal = pdf.getVal();
-      return pdf.normalizeWithNaNPacking(rawVal, normVal);
-   };
-};
-
 using std::endl, std::string, std::ostream;
 
 
@@ -718,8 +710,10 @@ std::vector<double> RooFFTConvPdf::scanPdf(RooRealVar &obs, RooAbsPdf &pdf, doub
   while(zeroBin<0) zeroBin+= N2 ;
 
   // To mimic exactly the normalization code in RooAbsPdf::getValV()
-  NaNPackingNormalizer npn;
-
+  auto getVal = [](RooAbsPdf const &p, double normVal) {
+     double rawVal = p.getVal();
+     return RooFit::Detail::normalizeWithNaNPacking(p, rawVal, normVal);
+  };
 
   // First scan hist into temp array
   std::vector<double> tmp(N2);
@@ -730,7 +724,7 @@ std::vector<double> RooFFTConvPdf::scanPdf(RooRealVar &obs, RooAbsPdf &pdf, doub
     // Sample entire extended range (N2 samples)
     for (k=0 ; k<N2 ; k++) {
       histX->setBin(k) ;
-      tmp[k] = npn.getVal(pdf, normVal);
+      tmp[k] = getVal(pdf, normVal);
     }
     break ;
 
@@ -739,16 +733,16 @@ std::vector<double> RooFFTConvPdf::scanPdf(RooRealVar &obs, RooAbsPdf &pdf, doub
     // bins with p.d.f. value at respective boundary
     {
       histX->setBin(0) ;
-      double val = npn.getVal(pdf, normVal);
+      double val = getVal(pdf, normVal);
       for (k=0 ; k<Nbuf ; k++) {
    tmp[k] = val ;
       }
       for (k=0 ; k<N ; k++) {
    histX->setBin(k) ;
-   tmp[k+Nbuf] = npn.getVal(pdf, normVal);
+   tmp[k+Nbuf] = getVal(pdf, normVal);
       }
       histX->setBin(N-1) ;
-      val = npn.getVal(pdf, normVal);
+      val = getVal(pdf, normVal);
       for (k=0 ; k<Nbuf ; k++) {
    tmp[N+Nbuf+k] = val ;
       }
@@ -760,13 +754,13 @@ std::vector<double> RooFFTConvPdf::scanPdf(RooRealVar &obs, RooAbsPdf &pdf, doub
     // bins with mirror image of sampled range
     for (k=0 ; k<N ; k++) {
       histX->setBin(k) ;
-      tmp[k+Nbuf] = npn.getVal(pdf, normVal);
+      tmp[k+Nbuf] = getVal(pdf, normVal);
     }
     for (k=1 ; k<=Nbuf ; k++) {
       histX->setBin(k) ;
-      tmp[Nbuf-k] = npn.getVal(pdf, normVal);
+      tmp[Nbuf-k] = getVal(pdf, normVal);
       histX->setBin(N-k) ;
-      tmp[Nbuf+N+k-1] = npn.getVal(pdf, normVal);
+      tmp[Nbuf+N+k-1] = getVal(pdf, normVal);
     }
     break ;
   }

roofit/roofitcore/src/RooFitImplHelpers.cxx

@@ -288,8 +288,7 @@ RooAbsArg *cloneTreeWithSameParametersImpl(RooAbsArg const &arg, RooArgSet const
 
 } // namespace RooHelpers
 
-namespace RooFit {
-namespace Detail {
+namespace RooFit::Detail {
 
 /// Transform a string into a valid C++ variable name by replacing forbidden
 /// characters with underscores.
@@ -335,8 +334,7 @@ std::string makeSliceCutString(RooArgSet const &sliceDataSet)
    return cutString.str();
 }
 
-} // namespace Detail
-} // namespace RooFit
+} // namespace RooFit::Detail
 
 namespace {
 

roofit/roofitcore/src/RooNormalizedPdf.cxx

@@ -13,6 +13,7 @@
 #include "RooFit/Detail/RooNormalizedPdf.h"
 
 #include "RooBatchCompute.h"
+#include "RooFitImplHelpers.h"
 
 #include <array>
 
@@ -51,4 +52,9 @@ void RooNormalizedPdf::doEval(RooFit::EvalContext &ctx) const
    }
 }
 
+double RooNormalizedPdf::getValV(const RooArgSet * /*normSet*/) const
+{
+   return normalizeWithNaNPacking(*_pdf, _pdf->getVal(), _normIntegral->getVal());
+}
+
 } // namespace RooFit::Detail