From 1d8b6a7d2491cf02f4b6f45fd9a514884b3d4ebc Mon Sep 17 00:00:00 2001
From: Jonas Rembser <jonas.rembser@cern.ch>
Date: Mon, 12 Apr 2021 11:48:35 +0200
Subject: [PATCH] [RF] Simplify constructors of test statistic classes
---
roofit/roofitcore/inc/RooAbsTestStatistic.h | 46 ++++-----
roofit/roofitcore/src/RooAbsTestStatistic.cxx | 93 ++-----------------
roofit/roofitcore/src/RooChi2Var.cxx | 9 +-
3 files changed, 34 insertions(+), 114 deletions(-)
diff --git a/roofit/roofitcore/inc/RooAbsTestStatistic.h b/roofit/roofitcore/inc/RooAbsTestStatistic.h
index f78ad2cf561..5274220bbb1 100644
--- a/roofit/roofitcore/inc/RooAbsTestStatistic.h
+++ b/roofit/roofitcore/inc/RooAbsTestStatistic.h
@@ -54,7 +54,7 @@ public:
};
// Constructors, assignment etc
- RooAbsTestStatistic() ;
+ RooAbsTestStatistic() {}
RooAbsTestStatistic(const char *name, const char *title, RooAbsReal& real, RooAbsData& data,
const RooArgSet& projDeps, Configuration const& cfg);
RooAbsTestStatistic(const RooAbsTestStatistic& other, const char* name=0);
@@ -116,14 +116,14 @@ protected:
}
// Original arguments
- RooAbsReal* _func ; // Pointer to original input function
- RooAbsData* _data ; // Pointer to original input dataset
- const RooArgSet* _projDeps ; // Pointer to set with projected observables
- std::string _rangeName ; // Name of range in which to calculate test statistic
- std::string _addCoefRangeName ; // Name of reference to be used for RooAddPdf components
- Bool_t _splitRange ; // Split rangeName in RooSimultaneous index labels if true
- Int_t _simCount ; // Total number of component p.d.f.s in RooSimultaneous (if any)
- Bool_t _verbose ; // Verbose messaging if true
+ RooAbsReal* _func = nullptr; // Pointer to original input function
+ RooAbsData* _data = nullptr; // Pointer to original input dataset
+ const RooArgSet* _projDeps = nullptr; // Pointer to set with projected observables
+ std::string _rangeName ; // Name of range in which to calculate test statistic
+ std::string _addCoefRangeName ; // Name of reference to be used for RooAddPdf components
+ Bool_t _splitRange = false; // Split rangeName in RooSimultaneous index labels if true
+ Int_t _simCount = 1; // Total number of component p.d.f.s in RooSimultaneous (if any)
+ Bool_t _verbose = false; // Verbose messaging if true
virtual Bool_t setDataSlave(RooAbsData& /*data*/, Bool_t /*cloneData*/=kTRUE, Bool_t /*ownNewDataAnyway*/=kFALSE) { return kTRUE ; }
@@ -136,27 +136,27 @@ protected:
void initSimMode(RooSimultaneous* pdf, RooAbsData* data, const RooArgSet* projDeps, std::string const& rangeName, std::string const& addCoefRangeName) ;
void initMPMode(RooAbsReal* real, RooAbsData* data, const RooArgSet* projDeps, std::string const& rangeName, std::string const& addCoefRangeName) ;
- mutable Bool_t _init ; //! Is object initialized
- GOFOpMode _gofOpMode ; // Operation mode of test statistic instance
+ mutable Bool_t _init = false; //! Is object initialized
+ GOFOpMode _gofOpMode = Slave; // Operation mode of test statistic instance
- Int_t _nEvents ; // Total number of events in test statistic calculation
- Int_t _setNum ; // Partition number of this instance in parallel calculation mode
- Int_t _numSets ; // Total number of partitions in parallel calculation mode
- Int_t _extSet ; //! Number of designated set to calculated extended term
+ Int_t _nEvents = 0; // Total number of events in test statistic calculation
+ Int_t _setNum = 0; // Partition number of this instance in parallel calculation mode
+ Int_t _numSets = 1; // Total number of partitions in parallel calculation mode
+ Int_t _extSet = 0; //! Number of designated set to calculated extended term
// Simultaneous mode data
- Int_t _nGof ; // Number of sub-contexts
- pRooAbsTestStatistic* _gofArray ; //! Array of sub-contexts representing part of the combined test statistic
+ Int_t _nGof = 0 ; // Number of sub-contexts
+ pRooAbsTestStatistic* _gofArray = nullptr; //! Array of sub-contexts representing part of the combined test statistic
std::vector<RooFit::MPSplit> _gofSplitMode ; //! GOF MP Split mode specified by component (when Auto is active)
// Parallel mode data
- Int_t _nCPU ; // Number of processors to use in parallel calculation mode
- pRooRealMPFE* _mpfeArray ; //! Array of parallel execution frond ends
+ Int_t _nCPU = 1; // Number of processors to use in parallel calculation mode
+ pRooRealMPFE* _mpfeArray = nullptr; //! Array of parallel execution frond ends
- RooFit::MPSplit _mpinterl ; // Use interleaving strategy rather than N-wise split for partioning of dataset for multiprocessor-split
- Bool_t _doOffset ; // Apply interval value offset to control numeric precision?
- mutable ROOT::Math::KahanSum<double> _offset{0.0} ; //! Offset as KahanSum to avoid loss of precision
- mutable Double_t _evalCarry; //! carry of Kahan sum in evaluatePartition
+ RooFit::MPSplit _mpinterl = RooFit::BulkPartition; // Use interleaving strategy rather than N-wise split for partioning of dataset for multiprocessor-split
+ Bool_t _doOffset = false; // Apply interval value offset to control numeric precision?
+ mutable ROOT::Math::KahanSum<double> _offset = 0.0; //! Offset as KahanSum to avoid loss of precision
+ mutable Double_t _evalCarry = 0.0; //! carry of Kahan sum in evaluatePartition
ClassDef(RooAbsTestStatistic,2) // Abstract base class for real-valued test statistics
diff --git a/roofit/roofitcore/src/RooAbsTestStatistic.cxx b/roofit/roofitcore/src/RooAbsTestStatistic.cxx
index 4c3e6682e79..a55c09e83b9 100644
--- a/roofit/roofitcore/src/RooAbsTestStatistic.cxx
+++ b/roofit/roofitcore/src/RooAbsTestStatistic.cxx
@@ -59,19 +59,6 @@ using namespace std;
ClassImp(RooAbsTestStatistic);
-////////////////////////////////////////////////////////////////////////////////
-/// Default constructor
-
-RooAbsTestStatistic::RooAbsTestStatistic() :
- _func(0), _data(0), _projDeps(0), _splitRange(0), _simCount(0),
- _verbose(kFALSE), _init(kFALSE), _gofOpMode(Slave), _nEvents(0), _setNum(0),
- _numSets(0), _extSet(0), _nGof(0), _gofArray(0), _nCPU(1), _mpfeArray(0),
- _mpinterl(RooFit::BulkPartition), _doOffset(kFALSE),
- _evalCarry(0)
-{
-}
-
-
////////////////////////////////////////////////////////////////////////////////
/// Create a test statistic from the given function and the data.
@@ -108,46 +95,15 @@ RooAbsTestStatistic::RooAbsTestStatistic(const char *name, const char *title, Ro
_rangeName(cfg.rangeName),
_addCoefRangeName(cfg.addCoefRangeName),
_splitRange(cfg.splitCutRange),
- _simCount(1),
_verbose(cfg.verbose),
- _nGof(0),
- _gofArray(0),
- _nCPU(cfg.nCPU),
- _mpfeArray(0),
- _mpinterl(cfg.interleave),
- _doOffset(kFALSE),
- _evalCarry(0)
+ // Determine if RooAbsReal is a RooSimultaneous
+ _gofOpMode{(cfg.nCPU>1 || cfg.nCPU==-1) ? MPMaster : (dynamic_cast<RooSimultaneous*>(_func) ? SimMaster : Slave)},
+ _nEvents{data.numEntries()},
+ _nCPU(cfg.nCPU != -1 ? cfg.nCPU : 1),
+ _mpinterl(cfg.interleave)
{
// Register all parameters as servers
- RooArgSet* params = real.getParameters(&data) ;
- _paramSet.add(*params) ;
- delete params ;
-
- if (_nCPU>1 || _nCPU==-1) {
-
- if (_nCPU==-1) {
- _nCPU=1 ;
- }
-
- _gofOpMode = MPMaster ;
-
- } else {
-
- // Determine if RooAbsReal is a RooSimultaneous
- Bool_t simMode = dynamic_cast<RooSimultaneous*>(&real)?kTRUE:kFALSE ;
-
- if (simMode) {
- _gofOpMode = SimMaster ;
- } else {
- _gofOpMode = Slave ;
- }
- }
-
- _setNum = 0 ;
- _extSet = 0 ;
- _numSets = 1 ;
- _init = kFALSE ;
- _nEvents = data.numEntries() ;
+ _paramSet.add(*std::unique_ptr<RooArgSet>{real.getParameters(&data)});
}
@@ -164,13 +120,12 @@ RooAbsTestStatistic::RooAbsTestStatistic(const RooAbsTestStatistic& other, const
_rangeName(other._rangeName),
_addCoefRangeName(other._addCoefRangeName),
_splitRange(other._splitRange),
- _simCount(1),
_verbose(other._verbose),
- _nGof(0),
- _gofArray(0),
+ // Determine if RooAbsReal is a RooSimultaneous
+ _gofOpMode{(other._nCPU>1 || other._nCPU==-1) ? MPMaster : (dynamic_cast<RooSimultaneous*>(_func) ? SimMaster : Slave)},
+ _nEvents{_data->numEntries()},
_gofSplitMode(other._gofSplitMode),
- _nCPU(other._nCPU),
- _mpfeArray(0),
+ _nCPU(other._nCPU != -1 ? other._nCPU : 1),
_mpinterl(other._mpinterl),
_doOffset(other._doOffset),
_offset(other._offset),
@@ -178,34 +133,6 @@ RooAbsTestStatistic::RooAbsTestStatistic(const RooAbsTestStatistic& other, const
{
// Our parameters are those of original
_paramSet.add(other._paramSet) ;
-
- if (_nCPU>1 || _nCPU==-1) {
-
- if (_nCPU==-1) {
- _nCPU=1 ;
- }
-
- _gofOpMode = MPMaster ;
-
- } else {
-
- // Determine if RooAbsReal is a RooSimultaneous
- Bool_t simMode = dynamic_cast<RooSimultaneous*>(_func)?kTRUE:kFALSE ;
-
- if (simMode) {
- _gofOpMode = SimMaster ;
- } else {
- _gofOpMode = Slave ;
- }
- }
-
- _setNum = 0 ;
- _extSet = 0 ;
- _numSets = 1 ;
- _init = kFALSE ;
- _nEvents = _data->numEntries() ;
-
-
}
diff --git a/roofit/roofitcore/src/RooChi2Var.cxx b/roofit/roofitcore/src/RooChi2Var.cxx
index 74b663f2d6f..952745180a3 100644
--- a/roofit/roofitcore/src/RooChi2Var.cxx
+++ b/roofit/roofitcore/src/RooChi2Var.cxx
@@ -75,7 +75,6 @@ namespace {
cfg.nCPU = RooCmdConfig::decodeIntOnTheFly("RooChi2Var::RooChi2Var","NumCPU",0,1,args...);
cfg.interleave = RooFit::Interleave;
cfg.verbose = static_cast<bool>(RooCmdConfig::decodeIntOnTheFly("RooChi2Var::RooChi2Var","Verbose",0,1,args...));
- cfg.splitCutRange = false;
cfg.cloneInputData = false;
cfg.integrateOverBinsPrecision = RooCmdConfig::decodeDoubleOnTheFly("RooChi2Var::RooChi2Var", "IntegrateBins", 0, -1., {args...});
return cfg;
@@ -83,15 +82,9 @@ namespace {
template<class ...Args>
RooAbsTestStatistic::Configuration makeRooAbsTestStatisticCfgForPdf(Args const& ... args) {
- RooAbsTestStatistic::Configuration cfg;
- cfg.rangeName = RooCmdConfig::decodeStringOnTheFly("RooChi2Var::RooChi2Var","RangeWithName",0,"",args...);
+ auto cfg = makeRooAbsTestStatisticCfgForFunc(args...);
cfg.addCoefRangeName = RooCmdConfig::decodeStringOnTheFly("RooChi2Var::RooChi2Var","AddCoefRange",0,"",args...);
- cfg.nCPU = RooCmdConfig::decodeIntOnTheFly("RooChi2Var::RooChi2Var","NumCPU",0,1,args...);
- cfg.interleave = RooFit::Interleave;
- cfg.verbose = static_cast<bool>(RooCmdConfig::decodeIntOnTheFly("RooChi2Var::RooChi2Var","Verbose",0,1,args...));
cfg.splitCutRange = static_cast<bool>(RooCmdConfig::decodeIntOnTheFly("RooChi2Var::RooChi2Var","SplitRange",0,0,args...));
- cfg.cloneInputData = false;
- cfg.integrateOverBinsPrecision = RooCmdConfig::decodeDoubleOnTheFly("RooChi2Var::RooChi2Var", "IntegrateBins", 0, -1., {args...});
return cfg;
}
}
--
GitLab