[RF] Change evaluatePartition interface for test statistics.

evaluatePartition() addresses event numbers with integers. It was changed to use std::size_t. This protects from negative indices, and is future proof when large numbers of events are available.
hageboeck · manolismih · Feb 5, 2019 · Feb 5, 2019 · Feb 6, 2019 · Feb 7, 2019
commit ba1a4fe8f6f5d4d7cdd9ee52d600ff1028582206
@@ -70,7 +70,7 @@ class RooAbsTestStatistic : public RooAbsReal {
   virtual Bool_t redirectServersHook(const RooAbsCollection& newServerList, Bool_t mustReplaceAll, Bool_t nameChange, Bool_t isRecursive) ;
   virtual Double_t evaluate() const ;
 
-  virtual Double_t evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const = 0 ;
+  virtual Double_t evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const = 0 ;
   virtual Double_t getCarry() const;
 
   void setMPSet(Int_t setNum, Int_t numSets) ; 

@@ -71,7 +71,7 @@ class RooChi2Var : public RooAbsOptTestStatistic {
   RooDataHist::ErrorType _etype ;     // Error type store in associated RooDataHist
   FuncMode _funcMode ;                // Function, P.d.f. or extended p.d.f?
 
-  virtual Double_t evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const ;
+  virtual Double_t evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const ;
 
   ClassDef(RooChi2Var,1) // Chi^2 function of p.d.f w.r.t a binned dataset
 };

@@ -49,7 +49,7 @@ class RooDataWeightedAverage : public RooAbsOptTestStatistic {
 
   Double_t _sumWeight ;  // Global sum of weights needed for normalization
   Bool_t _showProgress ; // Show progress indication during evaluation if true
-  virtual Double_t evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const ;
+  virtual Double_t evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const ;
 
   ClassDef(RooDataWeightedAverage,1) // Optimized calculator of data weighted average of a RooAbsReal
 };

@@ -65,7 +65,7 @@ class RooNLLVar : public RooAbsOptTestStatistic {
   static RooArgSet _emptySet ; // Supports named argument constructor
 
   Bool_t _extended ;
-  virtual Double_t evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const ;
+  virtual Double_t evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const;
   Bool_t _weightSq ; // Apply weights squared?
   mutable Bool_t _first ; //!
   Double_t _offsetSaveW2; //!

@@ -78,7 +78,7 @@ class RooXYChi2Var : public RooAbsOptTestStatistic {
   void initIntegrator() ;
   Double_t xErrorContribution(Double_t ydata) const ;
 
-  virtual Double_t evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const ;
+  virtual Double_t evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const ;
 
   RooNumIntConfig   _intConfig ; // Numeric integrator configuration for integration of function over bin
   RooAbsReal*       _funcInt ; //! Function integral

@@ -51,8 +51,8 @@ comparison to the `evaluate()` function.
 In addition, RooAbsPdf objects do not have a static concept of what
 variables are parameters and what variables are dependents (which
 need to be integrated over for a correct PDF normalization). 
-Instead the choice of normalization is always specified each time a
-normalized values is requested from the PDF via the getVal()
+Instead, the choice of normalization is always specified each time a
+normalized value is requested from the PDF via the getVal()
 method.
 
 RooAbsPdf manages the entire normalization logic of each PDF with
@@ -63,7 +63,7 @@ PDFs can advertise one or more (partial) analytical integrals of
 their function, and these will be used by RooRealIntegral, if it
 determines that this is safe (i.e. no hidden Jacobian terms,
 multiplication with other PDFs that have one or more dependents in
-commen etc)
+commen etc).
 
 #### Implementing analytical integrals
 To implement analytical integrals, two functions must be implemented. First,
@@ -76,19 +76,19 @@ is the set of dependents for which integration is requested. The
 function should copy the subset of dependents it can analytically
 integrate to `anaIntSet`, and return a unique identification code for
 this integration configuration.  If no integration can be
-performed, zero should be returned.  Second,
+performed, zero should be returned. Second,
 
 ```
 Double_t analyticalIntegral(Int_t code)
 ```
 
-Implements the actual analytical integral(s) advertised by
-getAnalyticalIntegral.  This functions will only be called with
-codes returned by getAnalyticalIntegral, except code zero.
+implements the actual analytical integral(s) advertised by
+`getAnalyticalIntegral()`.  This function will only be called with
+codes returned by `getAnalyticalIntegral()`, except code zero.
 
-The integration range for real each dependent to be integrated can
-be obtained from the dependents' proxy functions min() and
-max(). Never call these proxy functions for any proxy not known to
+The integration range for each dependent to be integrated can
+be obtained from the dependent's proxy functions `min()` and
+`max()`. Never call these proxy functions for any proxy not known to
 be a dependent via the integration code.  Doing so may be
 ill-defined, e.g. in case the proxy holds a function, and will
 trigger an assert. Integrated category dependents should always be

@@ -26,7 +26,7 @@ additional validation step where it also compares the numeric
 values of the tracked arguments with reference values to ensure
 that values have actually changed. This may be useful in case some
 of the tracked observables are in binned datasets where each
-observable propates a valueDirty flag when an event is loaded even
+observable propagates a valueDirty flag when an event is loaded even
 though usually only one observable actually changes.
 **/
 
@@ -131,7 +131,7 @@ RooChangeTracker::RooChangeTracker(const RooChangeTracker& other, const char* na
 
 
 ////////////////////////////////////////////////////////////////////////////////
-/// Returns true if state has changes since last call with clearState=kTRUE
+/// Returns true if state has changed since last call with clearState=kTRUE.
 /// If clearState is true, changeState flag will be cleared.
 
 Bool_t RooChangeTracker::hasChanged(Bool_t clearState) 

@@ -240,13 +240,12 @@ RooChi2Var::~RooChi2Var()
 ////////////////////////////////////////////////////////////////////////////////
 /// Calculate chi^2 in partition from firstEvent to lastEvent using given stepSize
 
-Double_t RooChi2Var::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const 
+Double_t RooChi2Var::evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
 {
   // Throughout the calculation, we use Kahan's algorithm for summing to
   // prevent loss of precision - this is a factor four more expensive than
   // straight addition, but since evaluating the PDF is usually much more
   // expensive than that, we tolerate the additional cost...
-  Int_t i ;
   Double_t result(0), carry(0);
 
   _dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize, kFALSE) ;
@@ -262,7 +261,7 @@ Double_t RooChi2Var::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t
 
   // Loop over bins of dataset
   RooDataHist* hdata = (RooDataHist*) _dataClone ;
-  for (i=firstEvent ; i<lastEvent ; i+=stepSize) {
+  for (auto i=firstEvent ; i<lastEvent ; i+=stepSize) {
 
     // get the data values for this event
     hdata->get(i);

@@ -103,9 +103,8 @@ Double_t RooDataWeightedAverage::globalNormalization() const
 ////////////////////////////////////////////////////////////////////////////////
 /// Calculate the data weighted average for events [firstEVent,lastEvent] with step size stepSize
 
-Double_t RooDataWeightedAverage::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const 
+Double_t RooDataWeightedAverage::evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
 {
-  Int_t i ;
   Double_t result(0) ;
 
   _dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize,kFALSE) ;
@@ -115,7 +114,7 @@ Double_t RooDataWeightedAverage::evaluatePartition(Int_t firstEvent, Int_t lastE
     cout.flush() ;
   }
 
-  for (i=firstEvent ; i<lastEvent ; i+=stepSize) {
+  for (auto i=firstEvent ; i<lastEvent ; i+=stepSize) {
 
     // get the data values for this event
     _dataClone->get(i);

@@ -245,13 +245,12 @@ void RooNLLVar::applyWeightSquared(Bool_t flag)
 /// If this an extended likelihood, the extended term is added to the return likelihood
 /// in the batch that encounters the event with index 0.
 
-Double_t RooNLLVar::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const
+Double_t RooNLLVar::evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
 {
   // Throughout the calculation, we use Kahan's algorithm for summing to
   // prevent loss of precision - this is a factor four more expensive than
   // straight addition, but since evaluating the PDF is usually much more
   // expensive than that, we tolerate the additional cost...
-  Int_t i ;
   Double_t result(0), carry(0);
 
   RooAbsPdf* pdfClone = (RooAbsPdf*) _funcClone ;
@@ -265,7 +264,7 @@ Double_t RooNLLVar::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t s
   // If pdf is marked as binned - do a binned likelihood calculation here (sum of log-Poisson for each bin)
   if (_binnedPdf) {
 
-    for (i=firstEvent ; i<lastEvent ; i+=stepSize) {
+    for (auto i=firstEvent ; i<lastEvent ; i+=stepSize) {
 
       _dataClone->get(i) ;
 
@@ -282,7 +281,7 @@ Double_t RooNLLVar::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t s
       if (mu<=0 && N>0) {
 
 	// Catch error condition: data present where zero events are predicted
-	logEvalError(Form("Observed %f events in bin %d with zero event yield",N,i)) ;
+	logEvalError(Form("Observed %f events in bin %lu with zero event yield",N,i)) ;
 
       } else if (fabs(mu)<1e-10 && fabs(N)<1e-10) {
 
@@ -310,7 +309,7 @@ Double_t RooNLLVar::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t s
 
   } else {
 
-    for (i=firstEvent ; i<lastEvent ; i+=stepSize) {
+    for (auto i=firstEvent ; i<lastEvent ; i+=stepSize) {
 
       _dataClone->get(i) ;
 
@@ -340,7 +339,7 @@ Double_t RooNLLVar::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t s
 
 	// Calculate sum of weights-squared here for extended term
 	Double_t sumW2(0), sumW2carry(0);
-	for (i=0 ; i<_dataClone->numEntries() ; i++) {
+	for (auto i=0 ; i<_dataClone->numEntries() ; i++) {
 	  _dataClone->get(i);
 	  Double_t y = _dataClone->weightSquared() - sumW2carry;
 	  Double_t t = sumW2 + y;
@@ -415,7 +414,7 @@ Double_t RooNLLVar::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t s
       _offsetCarry = carry;
     }
 
-    // Substract offset
+    // Subtract offset
     Double_t y = -_offset - (carry + _offsetCarry);
     Double_t t = result + y;
     carry = (t - result) - y;

@@ -384,7 +384,7 @@ Double_t RooXYChi2Var::fy() const
 ////////////////////////////////////////////////////////////////////////////////
 /// Calculate chi^2 in partition from firstEvent to lastEvent using given stepSize
 
-Double_t RooXYChi2Var::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_t stepSize) const 
+Double_t RooXYChi2Var::evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
 {
   Double_t result(0), carry(0);
 
@@ -393,7 +393,7 @@ Double_t RooXYChi2Var::evaluatePartition(Int_t firstEvent, Int_t lastEvent, Int_
 
   _dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize,kFALSE ) ;
 
-  for (Int_t i=firstEvent ; i<lastEvent ; i+=stepSize) {
+  for (auto i=firstEvent ; i<lastEvent ; i+=stepSize) {
 
     // get the data values for this event
     xydata->get(i);