Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RF] Fix skipping of zero weights in tests statistic caching #13639

Merged
merged 1 commit into from Sep 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions roofit/roofitcore/inc/RooAbsOptTestStatistic.h
Expand Up @@ -73,6 +73,7 @@ class RooAbsOptTestStatistic : public RooAbsTestStatistic {
virtual RooArgSet requiredExtraObservables() const { return RooArgSet() ; }
void optimizeCaching() ;
void optimizeConstantTerms(bool,bool=true) ;
void runRecalculateCache(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const override;

RooArgSet* _normSet = nullptr; ///< Pointer to set with observables used for normalization
RooArgSet* _funcCloneSet = nullptr; ///< Set owning all components of internal clone of input function
Expand All @@ -84,6 +85,7 @@ class RooAbsOptTestStatistic : public RooAbsTestStatistic {
TString _sealNotice ; ///< User-defined notice shown when reading a sealed likelihood
RooArgSet* _funcObsSet = nullptr; ///< List of observables in the pdf expression
RooArgSet _cachedNodes ; ///<! List of nodes that are cached as constant expressions
bool _skipZeroWeights = false; ///<! Whether to skip entries with weight zero in the evaluation

RooAbsReal* _origFunc = nullptr; ///< Original function
RooAbsData* _origData = nullptr; ///< Original data
Expand Down
3 changes: 3 additions & 0 deletions roofit/roofitcore/inc/RooAbsTestStatistic.h
Expand Up @@ -92,6 +92,9 @@ class RooAbsTestStatistic : public RooAbsReal {
virtual double evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const = 0 ;
virtual double getCarry() const;

// Overridden in cache-optimized test statistic
virtual void runRecalculateCache(std::size_t /*firstEvent*/, std::size_t /*lastEvent*/, std::size_t /*stepSize*/) const {}

void setMPSet(Int_t setNum, Int_t numSets) ;
void setSimCount(Int_t simCount) {
// Store total number of components p.d.f. of a RooSimultaneous in this component test statistic
Expand Down
12 changes: 9 additions & 3 deletions roofit/roofitcore/src/RooAbsOptTestStatistic.cxx
Expand Up @@ -114,6 +114,7 @@ RooAbsOptTestStatistic::RooAbsOptTestStatistic(const RooAbsOptTestStatistic &oth
: RooAbsTestStatistic(other, name),
_sealed(other._sealed),
_sealNotice(other._sealNotice),
_skipZeroWeights(other._skipZeroWeights),
_integrateBinsPrecision(other._integrateBinsPrecision)
{
// Don't do a thing in master mode
Expand Down Expand Up @@ -582,7 +583,7 @@ void RooAbsOptTestStatistic::optimizeConstantTerms(bool activate, bool applyTrac
_funcClone->findConstantNodes(*_dataClone->get(),_cachedNodes) ;

// Cache constant nodes with dataset - also cache entries corresponding to zero-weights in data when using BinnedLikelihood
_dataClone->cacheArgs(this,_cachedNodes,_normSet,!_funcClone->getAttribute("BinnedLikelihood")) ;
_dataClone->cacheArgs(this,_cachedNodes,_normSet, _skipZeroWeights);

// Put all cached nodes in AClean value caching mode so that their evaluate() is never called
for (auto cacheArg : _cachedNodes) {
Expand Down Expand Up @@ -693,8 +694,8 @@ bool RooAbsOptTestStatistic::setDataSlave(RooAbsData& indata, bool cloneData, bo
_data = _dataClone ;

// ReCache constant nodes with dataset
if (_cachedNodes.getSize()>0) {
_dataClone->cacheArgs(this,_cachedNodes,_normSet) ;
if (!_cachedNodes.empty()) {
_dataClone->cacheArgs(this,_cachedNodes,_normSet, _skipZeroWeights);
}

// Adjust internal event count
Expand Down Expand Up @@ -771,3 +772,8 @@ const char* RooAbsOptTestStatistic::cacheUniqueSuffix() const {
return Form("_%lx", _dataClone->uniqueId().value()) ;
}


void RooAbsOptTestStatistic::runRecalculateCache(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
{
_dataClone->store()->recalculateCache(_projDeps, firstEvent, lastEvent, stepSize, _skipZeroWeights);
}
1 change: 1 addition & 0 deletions roofit/roofitcore/src/RooAbsTestStatistic.cxx
Expand Up @@ -256,6 +256,7 @@ double RooAbsTestStatistic::evaluate() const
break ;
}

runRecalculateCache(nFirst, nLast, nStep);
double ret = evaluatePartition(nFirst,nLast,nStep);

if (numSets()==1) {
Expand Down
4 changes: 0 additions & 4 deletions roofit/roofitcore/src/RooChi2Var.cxx
Expand Up @@ -240,12 +240,8 @@ RooChi2Var::RooChi2Var(const RooChi2Var& other, const char* name) :

double RooChi2Var::evaluatePartition(std::size_t firstEvent, std::size_t lastEvent, std::size_t stepSize) const
{

double result(0), carry(0);

_dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize, false) ;


// Determine normalization factor depending on type of input function
double normFactor(1) ;
switch (_funcMode) {
Expand Down
5 changes: 0 additions & 5 deletions roofit/roofitcore/src/RooDataWeightedAverage.cxx
Expand Up @@ -111,8 +111,6 @@ double RooDataWeightedAverage::evaluatePartition(std::size_t firstEvent, std::si
{
double result(0) ;

_dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize,false) ;

if (setNum()==0 && _showProgress) {
ccoutP(Plotting) << "." ;
cout.flush() ;
Expand All @@ -130,6 +128,3 @@ double RooDataWeightedAverage::evaluatePartition(std::size_t firstEvent, std::si

return result ;
}



10 changes: 5 additions & 5 deletions roofit/roofitcore/src/RooNLLVar.cxx
Expand Up @@ -108,6 +108,7 @@ RooNLLVar::RooNLLVar(const char *name, const char* title, RooAbsPdf& pdf, RooAbs
pc.process(arg7) ; pc.process(arg8) ; pc.process(arg9) ;

_extended = pc.getInt("extended") ;
_skipZeroWeights = true;
}


Expand Down Expand Up @@ -157,6 +158,10 @@ RooNLLVar::RooNLLVar(const char *name, const char *title, RooAbsPdf& pdf, RooAbs
++biter ;
}
}

_skipZeroWeights = false;
} else {
_skipZeroWeights = true;
}
}

Expand Down Expand Up @@ -231,11 +236,6 @@ double RooNLLVar::evaluatePartition(std::size_t firstEvent, std::size_t lastEven

auto * pdfClone = static_cast<RooAbsPdf*>(_funcClone);

// cout << "RooNLLVar::evaluatePartition(" << GetName() << ") projDeps = " << (_projDeps?*_projDeps:RooArgSet()) << endl ;

_dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize, (_binnedPdf?false:true) ) ;



// If pdf is marked as binned - do a binned likelihood calculation here (sum of log-Poisson for each bin)
if (_binnedPdf) {
Expand Down
2 changes: 0 additions & 2 deletions roofit/roofitcore/src/RooXYChi2Var.cxx
Expand Up @@ -373,8 +373,6 @@ double RooXYChi2Var::evaluatePartition(std::size_t firstEvent, std::size_t lastE
// Loop over bins of dataset
RooDataSet* xydata = (RooDataSet*) _dataClone ;

_dataClone->store()->recalculateCache( _projDeps, firstEvent, lastEvent, stepSize,false ) ;

for (auto i=firstEvent ; i<lastEvent ; i+=stepSize) {

// get the data values for this event
Expand Down