Added new histograms to the BeamPixel application #19758
Conversation
|
A new Pull Request was created by @dinardo (Mauro) for master. It involves the following packages: DQM/BeamMonitor @vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @vanbesien, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
+1 |
cmsbuild
added
dqm-approved
fully-signed
and removed
dqm-pending
pending-signatures
labels
|
The tests are being triggered in jenkins. |
|
This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
| myGaussFit->SetParameter(1, Vx_X_Fit->getTH1()->GetMean()); | ||
| myGaussFit->SetParameter(2, Vx_X_Fit->getTH1()->GetRMS()); | ||
| minXfit = Vx_X_Fit->getTH1()->GetBinLowEdge(1); | ||
| for (int i = 0; i < Vx_X_Fit->getTH1()->GetNbinsX(); i++) |
There was a problem hiding this comment.
hi @dinardo - all the loops should be indexed by unsigned ints.
otherwise, have you handled the case of histograms with 0 entries?
|
Dear David,
|
|
i guess I miss the answer? |
|
I wrote the answer on the git thread "review", I guess you didn't receive it. Please let me know if you need further information.
|
|
it seems github doesn't get it either..at least not in my view of github. but this only answers 1/2 of my comment.
… On Jul 19, 2017, at 11:55 AM, Mauro ***@***.***> wrote:
I wrote the answer on the git thread "review", I guess you didn't receive it.
Here it is:
the protection are the assignments:
minXfit = Vx_X_Fit->getTH1()->GetBinLowEdge(1);
and
maxXfit = Vx_X_Fit->getTH1()->GetBinLowEdge(Vx_X_Fit->getTH1()->GetNbinsX());
So, even if the histogram is empy, the low and high range of the fit are the lowest and highest bins of the histogram.
Please let me know if you need further information.
• Mauro.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Hi,
|
|
ah - right...
… On Jul 19, 2017, at 12:06 PM, Mauro ***@***.***> wrote:
Hi,
I'm not sure why this could be a problem.
Since the loop's range are deduced from Vx_X_Fit->getTH1()->GetNbinsX(); I can not use unsigned int because GetNbinsX gives back an int.
• Mauro.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
coming back the empty histograms - what I naively read is that the fit min would correspond to the maximum bin location, and the fit max would correspond to the first bin location. That would not be good - maybe you can just skip the fitting of empty histograms completely? |
|
Hi David,
|
|
and what happens if min>max?
… On Jul 19, 2017, at 2:29 PM, Mauro ***@***.***> wrote:
Hi David,
even if the histogram has just one bin, the code assigns the same value to the fit range boundaries, so the range collapse in one point.
I just checked it, and if you fit a histogram which has one single bin with a fit function range whose minimum and maximum coincide, you don't get any error.
So, frankly I don't see the issue here. The code is self-protected.
• Mauro.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
ah - I misread things- the current code is fine for empty histograms. (even if we'd avoid some useless printouts and CPU by just not doing the fit in that case) |
|
+1 |
|
Hi David,
|
I've added three new histograms to monitor the "gaussianity" of the vertex x, y, z distributions.