[RF] Consistently do bin width correction when creating a RooHist from a TH1 #10070
Conversation
guitargeek
requested review from
lmoneta,
hageboeck and
Axel-Naumann
as code owners
|
Starting build on |
|
Build failed on ROOT-ubuntu2004/soversion. Failing tests: |
|
Build failed on mac1015/python3. Failing tests: |
guitargeek
force-pushed
the
roodatahist_issue_1
branch
from
e2cfff9 to
6e636bb
Compare
|
Starting build on |
| @@ -87,6 +87,11 @@ RooHist::RooHist(const TH1 &data, Double_t nominalBinWidth, Double_t nSigma, Roo | |||
| Bool_t correctForBinWidth, Double_t scaleFactor) : | |||
| TGraphAsymmErrors(), _nominalBinWidth(nominalBinWidth), _nSigma(nSigma), _rawEntries(-1) | |||
| { | |||
There was a problem hiding this comment.
I don't understand why we don't support this.
I think one can have both case, and being able to create a RooHist also without any bin width correction, depending on the passed option
There was a problem hiding this comment.
The point was to not change code behavior. So far, the bin width correction parameter was ignored for Poisson errors, and it might surprise users if it's now considered correctly. Since we don't need Poisson errors with no bin width correction anyway, it's safer to disable that possibility than to fix it.
The RooPlot constructor from TH1 had an option to disable the bin width correction, but this argument was not even used when the RooHist was created with Poisson errors, which is the default case. The Bin width correction was always done in this case, no matter what was the value of `correctForBinWith`. However, when SumW2 errors are used, the `correctForBinWith` parameter was actually considered! This caused inconsitent behavior, because when plotting a `RooDataHist` the `correctForBinWith` parameter is hardcoded to `false, meaning the bin width correction is done for Poisson errors but not for SumW2 errors. This commit fixes this behavior by following the precedent set by the more common Poisson error: the `correctForBinWith` parameter is consistently ignored, and an exception is thrown when it's false. Consequently, the `correctForBinWith` parameter is also not hardcoded to `false` in `RooDataHist::plotOn` anymore. The correct way to disable the bin width correction is to enable the interpretation as a density when importing a TH1 to a RooDataHist. This commit fixes a bug reported on the forum.
guitargeek
force-pushed
the
roodatahist_issue_1
branch
from
6e636bb to
0a9a36d
Compare
|
Starting build on |
|
Build failed on ROOT-debian10-i386/cxx14. Failing tests: |
|
Build failed on mac11/cxx17. Failing tests: |
[RF] Consistently do bin width correction when making RooHist from TH1
The RooPlot constructor from TH1 had an option to disable the bin width
correction, but this argument was not even used when the RooHist was
created with Poisson errors, which is the default case. The Bin width
correction was always done in this case, no matter what was the value of
correctForBinWith.However, when SumW2 errors are used, the
correctForBinWithparameterwas actually considered! This caused inconsitent behavior, because when
plotting a
RooDataHistthecorrectForBinWithparameter is hardcodedto `false, meaning the bin width correction is done for Poisson errors
but not for SumW2 errors.
This commit fixes this behavior by following the precedent set by the
more common Poisson error: the
correctForBinWithparameter isconsistently ignored, and an exception is thrown when it's false.
Consequently, the
correctForBinWithparameter is also not hardcoded tofalseinRooDataHist::plotOnanymore.The correct way to disable the bin width correction is to enable the
interpretation as a density when importing a TH1 to a RooDataHist.
This commit fixes a bug reported on the forum.