-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
DQM Pixel Phase 1: Several new Efficiency trend plots #34384
DQM Pixel Phase 1: Several new Efficiency trend plots #34384
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34384/23773
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-34384/23775
|
A new Pull Request was created by @SissonJ (Jack Sisson) for master. It involves the following packages: DQM/SiPixelPhase1Common @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@@ -546,6 +541,7 @@ void HistogramManager::executePerLumiHarvesting(DQMStore::IBooker& iBooker, | |||
} | |||
|
|||
void HistogramManager::loadFromDQMStore(SummationSpecification& s, Table& t, DQMStore::IGetter& iGetter) { | |||
//std::cout << "Running the loadFromDQMStore function" << 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.
part of the debugging changes? Perhaps to remove?
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!
#Barycenter plots | ||
from DQM.SiPixelPhase1Summary.SiPixelBarycenter_cfi import * | ||
|
||
from RecoPixelVertexing.PixelLowPtUtilities.ClusterShapeHitFilterESProducer_cfi import * | ||
from RecoLocalTracker.SiStripClusterizer.SiStripClusterChargeCut_cfi import * | ||
|
||
|
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.
The cfi is importing the module but SiPixelPhase1EfficiencyExtras is not added to the sequence, is this intentional?
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.
Everything in OfflineDQM_source_cff is imported to OfflineDQM_harvesting_cff:
from DQM.SiPixelPhase1Config.SiPixelPhase1OfflineDQM_source_cff import *
The sequence this module appears in is: siPixelPhase1OfflineDQM_harvesting
@@ -97,6 +92,7 @@ | |||
+ SiPixelPhase1TrackResidualsHarvester | |||
+ RunQTests_online | |||
+ SiPixelPhase1SummaryOnline | |||
+ SiPixelPhase1EfficiencyExtras |
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.
Since this is affecting Online DQM but we cannot test it at present at P5 because it runs 11_3_X, have you tested it on your side too? When would you need these plots in Online, for CRAFT in August? Thanks
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.
Checking with group conveners. Our development was in 11_1_X, but we haven't explicitly tested Online. The plots are not urgent for CRAFT.
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.
@jfernan2 in principle this can wait till CRAFT. Jacob is testing with the online client privately now.
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.
OK thanks. I was asking because this PR is in 12_0_X and 12_0_0 should be ready by the end of August, since CMSSW_12_0_0 is expected to be used in the LHC beam test scheduled by the last week of September. So I am afraid that not for CRAFT...
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 guess we should just backport it, no?
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.
@jfernan2 Removed this Harvester from online since it is only needed in offline!
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34384/23778
|
Pull request #34384 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again. |
please test |
<Notes on implementation> | ||
*/ | ||
// | ||
// Original Author: Duncan Leggat |
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.
@SissonJ is this accurate?
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.
Fixed!
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34384/24920
|
Pull request #34384 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again. |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d208a8/18099/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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Additions to DQM/SiPixelPhase1
Added new trend plots:
Related talks:
https://indico.cern.ch/event/1000808/contributions/4355849/attachments/2241277/3800219/TrackerDQM5_7_21.pdf
https://indico.cern.ch/event/934813/contributions/4103234/attachments/2142978/3611403/PixelEfficiencyTrendPlots_111320.pdf
Tested with runTheMatrix.py -l 136.892
Tested missing lumiVsLS protection with fake HLT runTheMatrix