Skip to content

Commit

Permalink
[RF] Remove warnings about large pdf values and just check for infinity
Browse files Browse the repository at this point in the history
Usually, when this warning happens, it's a false positive that annoys
users and they complain about it. Since the threshold is hardcoded, it
happens usually when your observable domains are narrow and the
probability density is high, i.e. when the order of magnitude of your
observable is small.

For example, when you have a uniform in the range 0.0 to 1.1e-6, you'll
get this warning for every pdf evaluation for no reason.

Or if you have a RooProdPdf, this can happen even more easily: the
product of two uniforms over the domain 0.0 to 1.1e-3 each will also
reach the 1e-6 threshold.

So it's better to only check for infinite values, which was probably the
intention anyway.
  • Loading branch information
guitargeek committed Oct 2, 2024
1 parent 75a6cea commit df66e78
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 12 deletions.
2 changes: 1 addition & 1 deletion roofit/batchcompute/res/RooBatchCompute.h
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ enum Computer {
struct ReduceNLLOutput {
double nllSum = 0.0;
double nllSumCarry = 0.0;
std::size_t nLargeValues = 0;
std::size_t nInfiniteValues = 0;
std::size_t nNonPositiveValues = 0;
std::size_t nNaNValues = 0;
};
Expand Down
8 changes: 4 additions & 4 deletions roofit/batchcompute/src/RooBatchCompute.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -242,15 +242,15 @@ namespace {

inline std::pair<double, double> getLog(double prob, ReduceNLLOutput &out)
{
if (std::abs(prob) > 1e6) {
out.nLargeValues++;
}

if (prob <= 0.0) {
out.nNonPositiveValues++;
return {std::log(prob), -prob};
}

if (std::isinf(prob)) {
out.nInfiniteValues++;
}

if (std::isnan(prob)) {
out.nNaNValues++;
return {prob, RooNaNPacker::unpackNaN(prob)};
Expand Down
10 changes: 5 additions & 5 deletions roofit/roofitcore/src/RooAbsPdf.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -185,16 +185,16 @@ namespace {
inline double getLog(double prob, RooAbsReal const *caller)
{

if (std::abs(prob) > 1e6) {
oocoutW(caller, Eval) << "RooAbsPdf::getLogVal(" << caller->GetName()
<< ") WARNING: top-level pdf has a large value: " << prob << std::endl;
}

if (prob < 0) {
caller->logEvalError("getLogVal() top-level p.d.f evaluates to a negative number");
return RooNaNPacker::packFloatIntoNaN(-prob);
}

if (std::isinf(prob)) {
oocoutW(caller, Eval) << "RooAbsPdf::getLogVal(" << caller->GetName()
<< ") WARNING: top-level pdf has an infinite value" << std::endl;
}

if (prob == 0) {
caller->logEvalError("getLogVal() top-level p.d.f evaluates to zero");

Expand Down
4 changes: 2 additions & 2 deletions roofit/roofitcore/src/RooNLLVarNew.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,9 @@ void RooNLLVarNew::doEval(RooFit::EvalContext &ctx) const
auto nllOut = RooBatchCompute::reduceNLL(config, probas, _weightSquared ? weightsSumW2 : weights,
_doBinOffset ? ctx.at(*_offsetPdf) : std::span<const double>{});

if (nllOut.nLargeValues > 0) {
if (nllOut.nInfiniteValues > 0) {
oocoutW(&*_pdf, Eval) << "RooAbsPdf::getLogVal(" << _pdf->GetName()
<< ") WARNING: top-level pdf has unexpectedly large values" << std::endl;
<< ") WARNING: top-level pdf has some infinite values" << std::endl;
}
for (std::size_t i = 0; i < nllOut.nNonPositiveValues; ++i) {
_pdf->logEvalError("getLogVal() top-level p.d.f not greater than zero");
Expand Down

0 comments on commit df66e78

Please sign in to comment.