Avoid fitting empy histograms in PhotonDataCertification #44064
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44064/39013
|
|
A new Pull Request was created by @guitargeek for master. It involves the following packages:
@antoniovagnerini, @cmsbuild, @tjavaid, @nothingface0, @syuvivida, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-124470/37633/summary.html Comparison SummarySummary:
|
In ROOT master, RooFit doesn't want to fit empty histograms anymore.
I have to think about a little bit about how I want to deal with this in
ROOT master, because such fits are often user logic errors. Also, in the
case of the `PhotonDataCertification::invMassZtest()`:
```c++
RooMsgService::instance().setGlobalKillBelow(RooFit::WARNING);
...
BreitWigner.fitTo(test, RooFit::Range(80, 100), RooFit::PrintLevel(-1000));
if (std::abs(mRes.getValV() - ZMass) < ZWidth) {
return 1.0;
} else if (std::abs(mRes.getValV() - ZMass) < gamma.getValV()) {
return 0.9;
} else {
return 0.0;
}
```
So far, RooFit just silently didn't do any fit if the `test` data was
empty. This results in the `mRes` value not being changed from the
default nominal `ZMass`, and then the function returns a summary value
of `1.0`, indicating that the peak was within the Z width. Does that
really make sense for an empty histogram?
That is maybe something to think about for the DQM experts. For now,
this commit simply keeps the old behavior, it just makes the
`if (sumEntries() > 0.)` check explicit. Like this, it will continue
working with ROOT `master`.
cmsbuild
added
tests-pending
code-checks-pending
and removed
tests-approved
code-checks-approved
labels
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44064/39026
|
|
Pull request #44064 was updated. @rvenditti, @antoniovagnerini, @cmsbuild, @tjavaid, @nothingface0, @syuvivida can you please check and sign again. |
|
Closing the PR in favor of a fix in ROOT: |
In ROOT master, RooFit doesn't want to fit empty histograms anymore.
I have to think about a little bit about how I want to deal with this in ROOT master, because such fits are often user logic errors. Also, in the case of the
PhotonDataCertification::invMassZtest():So far, RooFit just silently didn't do any fit if the
testdata was empty. This results in themResvalue not being changed from the default nominalZMass, and then the function returns a summary value of1.0, indicating that the peak was within the Z width. Does that really make sense for an empty histogram?That is maybe something to think about for the DQM experts. For now, this commit simply keeps the old behavior, it just makes the
if (sumEntries() > 0.)check explicit. Like this, it will continue working with ROOTmaster.This PR was made in response to cms-sw/cmsdist#9025 (comment).