New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add DRnR plots to PixelPhase1 DQM #34516
Conversation
} | ||
for (int j : {1, 2, 3, 4, 6, 7, 8, 9}) { | ||
if (me_x->getBinEntries(j, i) < minHits_){ | ||
std::cout<<"Barrel Bin "<<j<<","<<i<<" - Entries "<<me2_x->getBinEntries(j, i)<<std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no cout
please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
me2_y->setBinEntries(me2_y->getBin(j, i),1); | ||
} | ||
else{ | ||
std::cout<<"Endcap Bin "<<j<<","<<i<<" - Entries "<<me2_x->getBinEntries(j, i)<<std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no cout
please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
@@ -81,6 +83,14 @@ namespace { | |||
histo[RESIDUAL_X].fill(it.resXprime, id, &iEvent); | |||
histo[RESIDUAL_Y].fill(it.resYprime, id, &iEvent); | |||
|
|||
if (it.resXprimeErr!=0 && it.resXprimeErr!=0){ | |||
std::cout<<it.resXprime<<" - "<<it.resXprimeErr<<" - "<<it.resXprime/it.resXprimeErr<<std::endl; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no cout
please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34516/24018
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34516/24020
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
code-checks |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34516/24021
|
A new Pull Request was created by @arossi83 (Alessandro Rossi) for master. It involves the following packages:
@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
#include "FWCore/Framework/interface/EventSetup.h" | ||
#include "DataFormats/Common/interface/Handle.h" | ||
#include "FWCore/Framework/interface/LuminosityBlock.h" | ||
#include "FWCore/Framework/interface/ESHandle.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's not needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
~SiPixelPhase1ResidualsExtra() override; | ||
|
||
protected: | ||
// BeginJob |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove commented code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -81,6 +92,13 @@ namespace { | |||
histo[RESIDUAL_X].fill(it.resXprime, id, &iEvent); | |||
histo[RESIDUAL_Y].fill(it.resYprime, id, &iEvent); | |||
|
|||
if (it.resXprimeErr != 0 && it.resXprimeErr != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't understand the logic here, isn't this duplicated twice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arossi83 can you please clarify this, or remove the duplication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I just realize where the duplication is, obviously is a typo. I just fixed it!
if (it.resXprimeErr != 0 && it.resXprimeErr != 0) { | ||
histo[NormRes_X].fill(it.resXprime / it.resXprimeErr, id, &iEvent); | ||
histo[NormRes_Y].fill(it.resYprime / it.resYprimeErr, id, &iEvent); | ||
histo[DRNR_X].fill(it.resXprime / it.resXprimeErr, id, &iEvent); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is DRNR_X
different from NormRes_X
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No difference at this level, the differences are introduced in the harvesting step. I separate the plots here in order to have a proper histogram name on the DQMIO root file for DRnR. Summarizing: here DRNR_X is used to fill a 2D Profile which will show NormRes mean value for each bin, in the harvesting the content of this 2D profile is changed replacing the Mean value with RMS for each bin(module).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't the histogram be booked directly in the harvesting step?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle we can have the MEs booked and filled in the step3 but to do that we need to implement a method to evaluate RMS of single module in a running way (storing average, average of the square and number of entries) and be sure to be multithread and Concurrent LS safe. In the way things are implemented the 2DProfile ME take care of everything and the "cost" is to have an apparently duplicated entry on step3.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34516/24093
|
Pull request #34516 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again. |
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9f6227/17011/summary.html Comparison SummarySummary:
|
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy, @perrotta (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
PR description:
The main target of this PR is the addition of DRnR plots to the PixelPhase1 DQM. Everything has been packed inside a dedicated harvester and also other residuals plots creation has been moved here for consistency
PR validation:
Running wf 136.892 and standalone pixel online DQM client
if this PR is a backport please specify the original PR and why you need to backport that PR:
not a backport and no backport forseen