Skip to content

Commit

Permalink
[RF] Handle nullptr or empty norm set ambiguity in RooAbsReal::getVal
Browse files Browse the repository at this point in the history
The downsteam code, -- like RooAddPdf::getValV for example -- assume
that a nullptr is passed when no normalization is requested. The case of
an empty norm set is not handled correctly in RooAddPdf::getValV,
leading to wrong results.

However, some calling code passes an empty norm set to
RooAbsReal::getVal instead of a `nullptr` in an attempt to disable
normalization.

This commit suggests to solve this ambiguity at the highest possible
level: right at the beginning of RooAbsReal::getVal. If the
normalization set is empty, the pointer pointing to it will be set to
`nullptr`.

This fixes issue root-project#8307.
  • Loading branch information
guitargeek committed Jun 8, 2021
1 parent 941e9db commit 24a85a1
Showing 1 changed file with 15 additions and 1 deletion.
16 changes: 15 additions & 1 deletion roofit/roofitcore/inc/RooAbsReal.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ class RooAbsReal : public RooAbsArg {
/// These are integrated over their current ranges to compute the normalisation constant,
/// and the unnormalised result is divided by this value.
inline Double_t getVal(const RooArgSet* normalisationSet = nullptr) const {
// Sometimes, the calling code uses an empty RooArgSet to request evaluation
// without normalization set instead of following the `nullptr` convention.
// To remove this ambiguity which might not always be correctly handled in
// downstream code, we set `normalisationSet` to nullptr if it is pointing
// to an empty set.
if(normalisationSet && normalisationSet->empty()) {
normalisationSet = nullptr;
}
#ifdef ROOFIT_CHECK_CACHED_VALUES
return _DEBUG_getVal(normalisationSet);
#else
Expand All @@ -103,7 +111,13 @@ class RooAbsReal : public RooAbsArg {
}

/// Like getVal(const RooArgSet*), but always requires an argument for normalisation.
inline Double_t getVal(const RooArgSet& normalisationSet) const { return _fast ? _value : getValV(&normalisationSet) ; }
inline Double_t getVal(const RooArgSet& normalisationSet) const {
// Sometimes, the calling code uses an empty RooArgSet to request evaluation
// without normalization set instead of following the `nullptr` convention.
// To remove this ambiguity which might not always be correctly handled in
// downstream code, we set `normalisationSet` to nullptr if it is an empty set.
return _fast ? _value : getValV(normalisationSet.empty() ? nullptr : &normalisationSet) ;
}

virtual Double_t getValV(const RooArgSet* normalisationSet = nullptr) const ;

Expand Down

0 comments on commit 24a85a1

Please sign in to comment.