[HLT] add tau3mu trigger DQM #20026

rmanzoni · 2017-08-02T16:31:42Z

Adds 4 1D histograms and 1 2D histogram for each of 4 new HLT paths (800 bins in total) to the HLT DQM workspace, under: HLT/BPH/Tau3Mu/

Monitors the relevant distributions of the 3-muon candidates:

pt
mass
eta
phi
eta vs phi

The example post-harvesting DQM file (run on signal MC) can be found here
/afs/cern.ch/work/m/manzoni/public/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO_Tau3Mu.root

cmsbuild · 2017-08-02T16:32:02Z

A new Pull Request was created by @rmanzoni (Riccardo Manzoni) for master.

It involves the following packages:

DQMOffline/Trigger

@kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please review it and eventually sign? Thanks.
@battibass, @mtosi, @jhgoh, @calderona, @HuguesBrun, @trocino, @rociovilar this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

mtosi · 2017-08-02T16:45:06Z

tracked by #19142

mtosi · 2017-08-02T16:46:59Z

DQMOffline/Trigger/plugins/Tau3MuMonitor.h

+    MEbinning phi_binning_ ; // for the 1D tau phi histogram and 2D tau eta vs phi histogram
+    MEbinning mass_binning_; // for the 1D tau mass histogram
+
+    GenericTriggerEventFlag* genTriggerEventFlag_; // "is trigger fired?" flag


unique_ptr, please

then, shouldn't you have a GenericTriggerEventFlag for the numerator and an other one for the denominator ?

unique: ok.
I'm not doing efficiencies, only 'occupancies', so no den & num

ah, but you are not doing efficiency plot !!!!

mtosi · 2017-08-02T16:48:42Z

do you think is possible to exploit the general DQM file recentely developed ?
https://github.com/rmanzoni/cmssw/blob/a91726f91f5220a49d6aa94ddac87f6028342e94/DQMOffline/Trigger/plugins/TriggerDQMBase.h

mtosi · 2017-08-02T16:49:08Z

DQMOffline/Trigger/plugins/Tau3MuMonitor.cc

+
+  // tau 3 mu 1D pt
+  histname = "tau1DPt"; 
+  tau1DPt_ = ibooker.book1D(histname, "", pt_binning_.nbins, pt_binning_.xmin, pt_binning_.xmax);


would not be better to have a variable binning ?

to make it readable it would require to divide the entries in each bin by the bin width.
It's probably possible but it's added complication

rmanzoni · 2017-08-02T16:53:03Z

I followed the MET example, which inherits from DQMEDAnalyzer.
What's the trade between TriggerDQMBase and DQMEDAnalyzer?

mtosi · 2017-08-02T17:01:37Z

DQMOffline/Trigger/python/Tau3MuMonitor_cfi.py

+  nbins = cms.uint32( 40  ),
+  xmin  = cms.double(  0.5),
+  xmax  = cms.double(  3. ),
+)


what a strange binning width ....is it intended ?

yes, I "expect" a peak at 1.78 GeV and cut narrow around it

mtosi · 2017-08-02T17:02:49Z

DQMOffline/Trigger/python/Tau3MuMonitor_cfi.py

+
+hltTau3Mumonitoring.taus = cms.InputTag("hltTauPt10MuPts511Mass1p2to2p3Iso", "Taus") # 3-muon candidates
+
+hltTau3Mumonitoring.GenericTriggerEventPSet.andOr          = cms.bool( False ) # https://github.com/cms-sw/cmssw/blob/76d343005c33105be1e01b7b7278c07d753398db/CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc#L249


I would suggest you to add the selection based on DCS flag
just to not have events where the tracker has HVOFF, for instance (at tier0 no json file is used)

andOr set to False should ensure that all the four conditions listed here must be satisfied

cmssw/CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc

Line 251 in 76d3430

return ( acceptDcs( event ) && acceptGt( event ) && acceptL1( event, setup ) && acceptHlt( event ) );

so I guess the DCS condition comes as a consequence, correct?

mtosi · 2017-08-02T17:04:50Z

DQMOffline/Trigger/python/Tau3MuMonitor_cff.py

+
+HLT_Tau3Mu_Mu5_Mu1_TkMu1_Tau10_Monitoring                                             = hltTau3Mumonitoring.clone()
+HLT_Tau3Mu_Mu5_Mu1_TkMu1_Tau10_Monitoring.FolderName                                  = cms.string('HLT/BPH/Tau3Mu/Mu5_Mu1_TkMu1_Tau10')
+HLT_Tau3Mu_Mu5_Mu1_TkMu1_Tau10_Monitoring.taus                                        = cms.InputTag('hltTauPt10MuPts511Mass1p2to2p3Iso', 'Taus') # 3-muon candidates


uhmm, so are you going to have this module run on HLTMonitor PD ?
otherwise I doubt you will have the needed collection as input ....
if so, I think you need to update your JIRA ticket and request to add this collection to the output for the HLTMonitor PD and the other output used by relval, for instance

right. Good point

mtosi · 2017-08-02T17:06:33Z

DQMOffline/Trigger/plugins/Tau3MuMonitor.cc

+
+  // get ahold of the tau(3mu) collection
+  edm::Handle<reco::CompositeCandidateCollection> tauHandle;
+  iEvent.getByToken( tauToken_, tauHandle );


can you please add a check of validity / availability of the Handle ?
because if it is not available, the code will simply crash ... and indeed I'm expecting the integration test to crash because of this .....
did you try
runTheMatrix -l limited
?

yes, isValid is always good to check

mtosi · 2017-08-02T17:08:21Z

TriggerDQMBase has almost everything the METMonitor has, but the former is the template ;) anyhow, I see that your code is slightly different, therefore I think it is fine like it (module the needed changes) mia

…

On Wed, Aug 2, 2017 at 6:53 PM, Riccardo Manzoni ***@***.***> wrote: I followed the MET example, which inherits from DQMEDAnalyzer. What's the trade between TriggerDQMBase and DQMEDAnalyzer? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20026 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEt58ypAD-k_uVHLKeCzbstLOjrI_Jd9ks5sUKlxgaJpZM4OrWTC> .

mtosi · 2017-08-02T17:10:37Z

a resolution of 2.5/40 = 0.0625 GeV ? .. fine ....

…

On Wed, Aug 2, 2017 at 7:03 PM, Riccardo Manzoni ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In DQMOffline/Trigger/python/Tau3MuMonitor_cfi.py <#20026 (comment)>: > +) +hltTau3Mumonitoring.histoPSet.etaPSet = cms.PSet( + nbins = cms.uint32( 10 ), + xmin = cms.double(- 2.6), + xmax = cms.double( 2.6), +) +hltTau3Mumonitoring.histoPSet.phiPSet = cms.PSet( + nbins = cms.uint32( 10), + xmin = cms.double(-pi), + xmax = cms.double( pi), +) +hltTau3Mumonitoring.histoPSet.massPSet = cms.PSet( + nbins = cms.uint32( 40 ), + xmin = cms.double( 0.5), + xmax = cms.double( 3. ), +) yes, I "expect" a peak at 1.78 GeV and cut narrow around it — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20026 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEt58-sq_XL-EV_Etvcj8CW1KJmPKlMnks5sUKvxgaJpZM4OrWTC> .

mtosi · 2017-08-02T17:11:08Z

you have to specify the partitions

…

On Wed, Aug 2, 2017 at 7:05 PM, Riccardo Manzoni ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In DQMOffline/Trigger/python/Tau3MuMonitor_cfi.py <#20026 (comment)>: > + xmax = cms.double( 2.6), +) +hltTau3Mumonitoring.histoPSet.phiPSet = cms.PSet( + nbins = cms.uint32( 10), + xmin = cms.double(-pi), + xmax = cms.double( pi), +) +hltTau3Mumonitoring.histoPSet.massPSet = cms.PSet( + nbins = cms.uint32( 40 ), + xmin = cms.double( 0.5), + xmax = cms.double( 3. ), +) + +hltTau3Mumonitoring.taus = cms.InputTag("hltTauPt10MuPts511Mass1p2to2p3Iso", "Taus") # 3-muon candidates + +hltTau3Mumonitoring.GenericTriggerEventPSet.andOr = cms.bool( False ) # https://github.com/cms-sw/cmssw/blob/76d343005c33105be1e01b7b7278c07d753398db/CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc#L249 andOr set to False should ensure that all the four conditions listed here must be satisfied https://github.com/cms-sw/cmssw/blob/76d343005c33105be1e01b7b7278c0 7d753398db/CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc#L251 so I guess the DCS condition comes as a consequence, correct? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20026 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEt58-2bJ5DjfvD4w6pi7Qc2-48huHSBks5sUKxXgaJpZM4OrWTC> .

…itions

cmsbuild · 2017-08-03T10:11:25Z

Pull request #20026 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again.

rmanzoni · 2017-08-03T10:20:08Z

@mtosi I've updated the code following your suggestions.

Let me answer the other questions:

I don't understand the point with TriggerDQMBase: my module does not inherit from METMonitor, but from DQMEDAnalyzer, which should be the base class. In case I can move to TriggerDQMBase, just to understand
60 MeV resolution should be fine, if not too coarse (see the offline attached plot)

mtosi · 2017-08-03T10:33:11Z

@cmsbuild, please test

cmsbuild · 2017-08-03T10:33:25Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22041/console Started: 2017/08/03 12:34

cmsbuild · 2017-08-28T08:51:24Z

Pull request #20026 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again.

cmsbuild · 2017-08-28T08:52:18Z

The code-checks are being triggered in jenkins.

cmsbuild · 2017-08-28T08:52:19Z

Pull request #20026 was updated. @kmaeshima, @cmsbuild, @vanbesien, @vazzolini, @dmitrijus can you please check and sign again.

cmsbuild · 2017-08-28T08:54:35Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20026/338

mtosi · 2017-08-28T09:28:23Z

please, let me know when you will be done w/ the updates so, we can ask for the integration test ;)

…

On Mon, Aug 28, 2017 at 10:54 AM, cmsbuild ***@***.***> wrote: +code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-20026/338 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20026 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEt58xskgVb7JvdQEScN3MXwGFUucbBiks5scoBNgaJpZM4OrWTC> .

rmanzoni · 2017-08-28T09:44:59Z

if there's no further comments, I'm fine with the current status, you can trigger the tests

mtosi · 2017-08-28T10:58:10Z

@cmsbuild, please test

cmsbuild · 2017-08-28T10:58:29Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/22519/console Started: 2017/08/28 12:58

cmsbuild · 2017-08-28T13:07:59Z

+1
Tested at: f412c60
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20026/22519/summary.html

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
85b5c53
5ca2c40
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20026/22519/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20026/22519/git-merge-result

cmsbuild · 2017-08-28T13:08:02Z

Comparison job queued.

cmsbuild · 2017-08-28T14:45:04Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-20026/22519/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 26
DQMHistoTests: Total histograms compared: 2653934
DQMHistoTests: Total failures: 193
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2653552
DQMHistoTests: Total skipped: 189
DQMHistoTests: Total Missing objects: 0
Checked 107 log files, 14 edm output root files, 26 DQM output files

dmitrijus · 2017-08-31T13:47:21Z

+1

cmsbuild · 2017-08-31T13:47:42Z

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. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2)

davidlange6 · 2017-09-03T15:35:06Z

+1

rmanzoni added 2 commits August 2, 2017 18:19

add tau3mu DQM

a30a9f7

remove testing switches

a91726f

cmsbuild added this to the CMSSW_9_3_X milestone Aug 2, 2017

cmsbuild added comparison-pending dqm-pending orp-pending pending-signatures tests-pending labels Aug 2, 2017

rmanzoni mentioned this pull request Aug 2, 2017

[HLT] add tau3mu trigger DQM #20027

Closed

mtosi mentioned this pull request Aug 2, 2017

HLT DQM #19142

Open

mtosi reviewed Aug 2, 2017

View reviewed changes

rmanzoni added 2 commits August 3, 2017 11:18

Merge branch 'master' into master_tau3muDQM

305a2db

check if the handle is valid, move to smart pointers, define DCS part…

90d8d52

…itions

cmsbuild removed the tests-pending label Aug 3, 2017

improve guard, remove unnecessary includes

f412c60

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 28, 2017

cmsbuild added tests-started and removed tests-pending labels Aug 28, 2017

cmsbuild added tests-approved and removed tests-started labels Aug 28, 2017

cmsbuild added comparison-available and removed comparison-pending labels Aug 28, 2017

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Aug 31, 2017

cmsbuild added orp-approved and removed orp-pending labels Sep 3, 2017

cmsbuild merged commit 67b29c7 into cms-sw:master Sep 3, 2017


		hltTau3Mumonitoring.taus = cms.InputTag("hltTauPt10MuPts511Mass1p2to2p3Iso", "Taus") # 3-muon candidates

		hltTau3Mumonitoring.GenericTriggerEventPSet.andOr = cms.bool( False ) # https://github.com/cms-sw/cmssw/blob/76d343005c33105be1e01b7b7278c07d753398db/CommonTools/TriggerUtils/src/GenericTriggerEventFlag.cc#L249

[HLT] add tau3mu trigger DQM #20026

[HLT] add tau3mu trigger DQM #20026

Conversation

rmanzoni commented Aug 2, 2017

cmsbuild commented Aug 2, 2017

mtosi commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mtosi commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmanzoni commented Aug 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmanzoni Aug 2, 2017 • edited

Choose a reason for hiding this comment

mtosi commented Aug 2, 2017 via email

mtosi commented Aug 2, 2017 via email

mtosi commented Aug 2, 2017 via email

cmsbuild commented Aug 3, 2017

rmanzoni commented Aug 3, 2017

mtosi commented Aug 3, 2017

cmsbuild commented Aug 3, 2017 • edited

cmsbuild commented Aug 28, 2017

cmsbuild commented Aug 28, 2017

cmsbuild commented Aug 28, 2017

cmsbuild commented Aug 28, 2017

mtosi commented Aug 28, 2017 via email

rmanzoni commented Aug 28, 2017

mtosi commented Aug 28, 2017

cmsbuild commented Aug 28, 2017 • edited

cmsbuild commented Aug 28, 2017

cmsbuild commented Aug 28, 2017

cmsbuild commented Aug 28, 2017

dmitrijus commented Aug 31, 2017

cmsbuild commented Aug 31, 2017

davidlange6 commented Sep 3, 2017

rmanzoni Aug 2, 2017 •

edited

cmsbuild commented Aug 3, 2017 •

edited

cmsbuild commented Aug 28, 2017 •

edited