[RF] Zero out negative bins in RooHistPdf for consistent normalization - #23103
[RF] Zero out negative bins in RooHistPdf for consistent normalization#23103guitargeek wants to merge 1 commit into
Conversation
ae72e31 to
d2d9854
Compare
| for (std::size_t i = 0; i < nBins; ++i) { | ||
| if (dh->weight(i) < 0.) { | ||
| // Keep the bin error, which still quantifies the statistical | ||
| // uncertainty of the original bin content estimate. |
There was a problem hiding this comment.
While I agree on this statement, I don't fully get why we need to preserve this uncertainty: can an example be made of a case where this quantity is needed?
There was a problem hiding this comment.
Very good question! As far as I remember, there is no place in RooFit where we need to get the weight error for a RooHistPdf.
However, there is a public interface of the RooHistPdf to get the underlying RooDataHist, so in principle users could rely on the uncertainties being present in the RooDataHist. I could think of a usecase: if you'd like to get out the original statistical uncertainty of an MC template, for example to reason if the sub-zero value could be a statistical fluctuation. A bit of a constructed example though. In any case, I'll add a comment!
There was a problem hiding this comment.
Added a comment!
There was a problem hiding this comment.
thanks a lot for the clarification. The example is maybe a niche one, but it's a use case.
A RooHistPdf clamps negative bin contents to zero when evaluating, but its normalization integral was based on the plain sum of the bin weights, including the negative ones. A histogram with negative bins therefore yielded a pdf that did not integrate to unity, silently biasing in particular the yields of extended fits: even an Asimov fit with a model identical to the generating pdf did not close. Now, if the input histogram contains bins with negative content, these are set to zero in an internally-owned clone of the histogram that is used instead, with a warning. This makes the pdf value and its normalization consistent, restoring exact closure. The input histogram is not modified, and bin errors are preserved. The RooHistPdf constructed by RooAbsCachedPdf implementations like RooFFTConvPdf is unaffected, because the cache histogram is still empty at construction time and only filled (in place) afterwards. Fixes ROOT-10825. 🤖 Done with the help of AI
d2d9854 to
773da6b
Compare
Test Results 23 files 23 suites 3d 16h 32m 35s ⏱️ For more details on these failures, see this check. Results for commit 773da6b. |
2 participants
A RooHistPdf clamps negative bin contents to zero when evaluating, but its normalization integral was based on the plain sum of the bin weights, including the negative ones. A histogram with negative bins therefore yielded a pdf that did not integrate to unity, silently biasing in particular the yields of extended fits: even an Asimov fit with a model identical to the generating pdf did not close.
Now, if the input histogram contains bins with negative content, these are set to zero in an internally-owned clone of the histogram that is used instead, with a warning. This makes the pdf value and its normalization consistent, restoring exact closure. The input histogram is not modified, and bin errors are preserved.
The RooHistPdf constructed by RooAbsCachedPdf implementations like RooFFTConvPdf is unaffected, because the cache histogram is still empty at construction time and only filled (in place) afterwards.
Fixes ROOT-10825.
🤖 Done with the help of AI