Add offline DQM for MET plus track EXO triggers #19490

bpfrancis · 2017-06-29T19:55:48Z

Creates offline DQM monitoring for the MET+Track triggers for the 2017 pp collisions. These paths are owned by Exotica and are in the MET primary dataset.

c.f. TSG approval presentation: https://indico.cern.ch/event/646747/contributions/2626978/attachments/1476569/2287752/bfrancis_OSU_TSG.pdf

and JIRA ticket: https://its.cern.ch/jira/browse/CMSHLT-1381

cmsbuild · 2017-06-29T19:56:13Z

A new Pull Request was created by @bpf6qc (Brian Francis) for master.

It involves the following packages:

DQMOffline/Trigger

@vazzolini, @kmaeshima, @dmitrijus, @cmsbuild, @vanbesien, @davidlange6 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-07-01T12:16:30Z

why couldn't you make use of the already available code ?
if related to the matching you are applying between the online and offline muon ?
couldn't you add it to the available METMonitor or TopMonitor ?

then, try to squeeze as much as possible the # bins
and please, quantify the total # bins added by your PR

mtosi · 2017-07-01T12:16:54Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+  , hltTrk50FilterTag_       ( iConfig.getParameter<edm::InputTag>("isoTrkFilter") )
+  , met_variable_binning_    ( iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<std::vector<double> >("metBinning") )
+  , muonPt_variable_binning_ ( iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<std::vector<double> >("ptBinning")  )
+  , met_binning_          ( getHistoPSet   (iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<edm::ParameterSet>("metPSet")    ) )


do you really need it ?

Which of these are you referring to? If you mean this variable binning histograms, we don't particularly need them; it seemed to be a common inclusion from other modules.

mtosi · 2017-07-01T12:17:18Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+  , jetToken_             ( consumes<reco::PFJetCollection>        (iConfig.getParameter<edm::InputTag>("jets")           ) )
+  , vtxToken_             ( consumes<reco::VertexCollection>       (iConfig.getParameter<edm::InputTag>("vertices")       ) )
+  , theTrigSummary_       ( consumes<trigger::TriggerEvent>        (iConfig.getParameter<edm::InputTag>("trigSummary")    ) )
+  , hltTrk50FilterTag_       ( iConfig.getParameter<edm::InputTag>("isoTrkFilter") )


you might want to have such parameter slightly more generic ?

This will be changed.

mtosi · 2017-07-01T12:17:34Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+  , muonPt_variable_binning_ ( iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<std::vector<double> >("ptBinning")  )
+  , met_binning_          ( getHistoPSet   (iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<edm::ParameterSet>("metPSet")    ) )
+  , ls_binning_           ( getHistoLSPSet (iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<edm::ParameterSet>("lsPSet")     ) )
+  , pt_binning_           ( getHistoPSet   (iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<edm::ParameterSet>("ptPSet")     ) )


do you need it ?

Same question as above: if you mean the variable binning, no.

mtosi · 2017-07-01T12:18:25Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+   bookME(ibooker, mTmuonMetME_, histname, histtitle, pt_binning_.nbins, pt_binning_.xmin, pt_binning_.xmax);
+   setMETitle(mTmuonMetME_, "mT(Muon, CaloMET) [GeV]", "events / [GeV]");
+
+   histname = "muonPtVsLS"; histtitle = "Muon PT vs LS";


you have already the MET vs LS, are you applying a different selection for this ?
if not, please drop

This was requested by an analyzer, but you're right there is no additional selection: I'll remove it.

mtosi · 2017-07-01T12:18:46Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+   histname = "muonEta"; histtitle = "Muon eta";
+   bookME(ibooker, muonEtaME_, histname, histtitle, eta_binning_.nbins, eta_binning_.xmin, eta_binning_.xmax);
+   setMETitle(muonEtaME_, "Muon #eta", "events / 0.2");
+


I think you are missing the eta-phi plot

I will add it.

mtosi · 2017-07-01T12:19:20Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+  std::vector<reco::Muon> muons;
+  muons.clear();
+  for(auto const & m : *muonHandle) {
+    bool pass = m.isGlobalMuon() &&


couldn't you make use of some of the functions provided by MUO POG ?

Yes; this is just reco::Muon::isTight and will be changed. Also I realize here that I haven't included muon isolation, which we want to require.

mtosi · 2017-07-01T15:35:18Z

the other way round ;)

…

On 1 Jul 2017, at 16:49, Brian Francis ***@***.***> wrote: @bpf6qc commented on this pull request. In DQMOffline/Trigger/plugins/METplusTrackMonitor.cc: > +// constructors and destructor +// ----------------------------- + +METplusTrackMonitor::METplusTrackMonitor( const edm::ParameterSet& iConfig ) : + folderName_ ( iConfig.getParameter<std::string>("FolderName") ) + , metToken_ ( consumes<reco::CaloMETCollection> (iConfig.getParameter<edm::InputTag>("met") ) ) + , hltMetToken_ ( consumes<reco::CaloMETCollection> (iConfig.getParameter<edm::InputTag>("onlineMet") ) ) + , hltMetCleanToken_ ( consumes<reco::CaloMETCollection> (iConfig.getParameter<edm::InputTag>("onlineMetClean") ) ) + , muonToken_ ( consumes<reco::MuonCollection> (iConfig.getParameter<edm::InputTag>("muons") ) ) + , jetToken_ ( consumes<reco::PFJetCollection> (iConfig.getParameter<edm::InputTag>("jets") ) ) + , vtxToken_ ( consumes<reco::VertexCollection> (iConfig.getParameter<edm::InputTag>("vertices") ) ) + , theTrigSummary_ ( consumes<trigger::TriggerEvent> (iConfig.getParameter<edm::InputTag>("trigSummary") ) ) + , hltTrk50FilterTag_ ( iConfig.getParameter<edm::InputTag>("isoTrkFilter") ) + , met_variable_binning_ ( iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<std::vector<double> >("metBinning") ) + , muonPt_variable_binning_ ( iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<std::vector<double> >("ptBinning") ) + , met_binning_ ( getHistoPSet (iConfig.getParameter<edm::ParameterSet>("histoPSet").getParameter<edm::ParameterSet>("metPSet") ) ) Which of these are you referring to? If you mean this variable binning histograms, we don't particularly need them; it seemed to be a common inclusion from other modules. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

mtosi · 2017-07-01T15:35:58Z

;)

…

On 1 Jul 2017, at 17:13, Brian Francis ***@***.***> wrote: @bpf6qc commented on this pull request. In DQMOffline/Trigger/plugins/METplusTrackMonitor.cc: > + if(!primaryVertices->size()) return; + const reco::Vertex* pv = nullptr; + for(auto const& v: *primaryVertices) { + if (!vtxSelection_(v)) continue; + pv = &v; + break; + } + if(pv == nullptr) return; + + edm::Handle<reco::MuonCollection> muonHandle; + iEvent.getByToken(muonToken_, muonHandle); + if(muonHandle->size() < nmuons_) return; + std::vector<reco::Muon> muons; + muons.clear(); + for(auto const & m : *muonHandle) { + bool pass = m.isGlobalMuon() && Yes; this is just reco::Muon::isTight and will be changed. Also I realize here that I haven't included muon isolation, which we want to require. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

bpfrancis · 2017-07-01T16:53:47Z

Okay, I understand for the plot binnings. We always report our efficiencies with log-x (variable) binning so those binnings are preferred anyway.

To answer your question about why this is its own module, the METMonitor would need very significant additions that the MET paths would never use, and the TOPMonitor conversely monitors much more than we require as well as not having the machinery to measure a multi-leg efficiency with online requirements and online/offline matching. In the end I felt it more appropriate to write this than to disturb those two so severely.

cmsbuild · 2017-07-01T18:56:29Z

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

bpfrancis · 2017-07-01T19:03:26Z

I've tried to address all the issues mentioned. I also have included a requirement for tight PF-based muon isolation, which was mistakenly left out before.

Total bins: 37,440 per path --> 74,880 total.
Of these about 40% are the number of bins in the versus-LS plots (2 paths * 2 types (met, muon) * 3 (num/den/eff) * 2500 bins each), where 2500 was chosen just because other modules seem to use that number of bins.

fwyzard · 2017-07-03T08:57:07Z

@cmsbuild , please test

cmsbuild · 2017-07-03T08:57:26Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21072/console Started: 2017/07/03 10:58

fwyzard · 2017-07-03T09:00:22Z

tracked by #19142

cmsbuild · 2017-07-03T10:45:23Z

+1
Tested at: 1913500
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19490/21072/summary.html

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

cmsbuild · 2017-07-03T10:45:25Z

Comparison job queued.

cmsbuild · 2017-07-13T08:49:32Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21424/console Started: 2017/07/13 10:50

cmsbuild · 2017-07-13T08:49:33Z

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

fwyzard · 2017-07-13T08:51:52Z

Please do not rebase this.
As soon as @dmitrijus (done) and @davidlange6 let me know that the PR is fine (apart from the conflict) I will fix it in a separate PR.

cmsbuild · 2017-07-13T08:52:34Z

-1
@bpf6qc This pull request cannot be automatically merged, could you please rebase it?

You can see the log for git cms-merge-topic here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19490/21424/git-merge-result

fwyzard · 2017-07-23T16:27:15Z

@davidlange6, can you comment on this PR ?

davidlange6 · 2017-07-24T08:19:28Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+  iEvent.getByToken(jetToken_, jetsHandle);
+  if(jetsHandle->size() < njets_) return;
+  std::vector<reco::PFJet> jets;
+  jets.clear();


hi @bpf6qc - this clear is not needed.

davidlange6 · 2017-07-24T08:19:56Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+    if(jetSelection_(j)) jets.push_back(j);
+  }
+  if(jets.size() < njets_) return;
+  if(njets_ > 0 && jets.size() > 0 && fabs(jets[0].eta()) > leadJetEtaCut_) return;


please use jets.empty()

If you mean jets.clear() then I agree, I hadn't deallocated them before.

No, really jets.empty(): not empty() is more efficient than size() > 0

davidlange6 · 2017-07-24T08:20:53Z

DQMOffline/Trigger/plugins/METplusTrackMonitor.cc

+  metVsHltMetClean_.numerator->Fill(hltMetClean.pt(), met);
+
+  // Filling track leg histograms (denominator)
+  double leadMuonPt = muons.size() ? muons[0].pt() : -1.0;


muons.empty()

Same as above, if you mean muons.clear() then okay, otherwise empty() just returns a bool.

not muons.empty() ? ... : ... is equivalent to and more efficient than muons.size() ? ... : ...

fwyzard · 2017-07-24T20:09:59Z

@bpf6qc can you address the comments from David ?

fwyzard · 2017-07-25T08:29:59Z

@bpf6qc can you push an update to this PR ?

cmsbuild · 2017-07-25T13:18:37Z

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

Fixed conflicts: DQMOffline/Trigger/python/ExoticaMonitoring_Client_cff.py DQMOffline/Trigger/python/ExoticaMonitoring_cff.py

fwyzard · 2017-07-26T00:25:11Z

I have resolved the conflicts in #19916.
Please DO NOT TOUCH this PR/branch, and leave it open.

This PR includes a backport of the following PRs: - cms-sw#18172 - cms-sw#18950 - cms-sw#18959 - cms-sw#18968 - cms-sw#18971 - cms-sw#19023 - cms-sw#19046 - cms-sw#19078 - cms-sw#19119 - cms-sw#19178 - cms-sw#19290 - cms-sw#19293 - cms-sw#19294 - cms-sw#19490 - cms-sw#19499 - cms-sw#19577 - cms-sw#19585 - cms-sw#19596 - cms-sw#19599 - cms-sw#19627 - cms-sw#19689 - cms-sw#19694 - cms-sw#19703 - cms-sw#19781 - cms-sw#19794 plus the older ones, contained in DQMOffline/Trigger and HLTriggerOffline. It synchronises with CMSSW_9_3_X - DQMServices/ClientConfig - DQMOffline/Configuration - DQMOffline/Trigger - HLTriggerOffline/Btag - HLTriggerOffline/Higgs - HLTriggerOffline/SUSYBSM - HLTriggerOffline/Tau - HLTriggerOffline/Top

Add offline DQM for MET plus track EXO triggers

e3f918c

cmsbuild added this to the CMSSW_9_3_X milestone Jun 29, 2017

cmsbuild added comparison-pending dqm-pending orp-pending pending-signatures tests-pending labels Jun 29, 2017

mtosi reviewed Jul 1, 2017

View reviewed changes

mtosi mentioned this pull request Jul 1, 2017

HLT DQM #19142

Open

Adjust binning, histograms, and some variable names as requested.

1913500

cmsbuild added tests-started and removed tests-pending labels Jul 3, 2017

cmsbuild removed the tests-started label Jul 3, 2017

cmsbuild added the fully-signed label Jul 13, 2017

cmsbuild added tests-started and removed tests-pending labels Jul 13, 2017

davidlange6 reviewed Jul 24, 2017

View reviewed changes

Resolving comments about vector clear() and size()

38afbc3

cmsbuild added dqm-pending pending-signatures tests-pending and removed dqm-approved fully-signed tests-started labels Jul 25, 2017

fwyzard added a commit to fwyzard/cmssw that referenced this pull request Jul 25, 2017

Merge pull request cms-sw#19490 from bpf6qc:METplusTrackOfflineDQM92X

fa3d004

Fixed conflicts: DQMOffline/Trigger/python/ExoticaMonitoring_Client_cff.py DQMOffline/Trigger/python/ExoticaMonitoring_cff.py

fwyzard mentioned this pull request Jul 25, 2017

HLT DQM for 9.3.x - part 4 #19916

Merged

cmsbuild merged commit 38afbc3 into cms-sw:master Jul 28, 2017

fwyzard mentioned this pull request Jul 28, 2017

Enable HLT DQM for MET plus Track EXO triggers #19946

Merged

fwyzard mentioned this pull request Jul 28, 2017

backport HLT DQM for menu v2.1 to CMSSW 9.2.x #19961

Closed

Add offline DQM for MET plus track EXO triggers #19490

Add offline DQM for MET plus track EXO triggers #19490

Conversation

bpfrancis commented Jun 29, 2017

cmsbuild commented Jun 29, 2017

mtosi commented Jul 1, 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

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

mtosi commented Jul 1, 2017 via email

mtosi commented Jul 1, 2017 via email

bpfrancis commented Jul 1, 2017

cmsbuild commented Jul 1, 2017

bpfrancis commented Jul 1, 2017

fwyzard commented Jul 3, 2017

cmsbuild commented Jul 3, 2017 • edited Loading

fwyzard commented Jul 3, 2017

cmsbuild commented Jul 3, 2017

cmsbuild commented Jul 3, 2017

cmsbuild commented Jul 13, 2017 • edited Loading

cmsbuild commented Jul 13, 2017

fwyzard commented Jul 13, 2017

cmsbuild commented Jul 13, 2017

fwyzard commented Jul 23, 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

Choose a reason for hiding this comment

fwyzard commented Jul 24, 2017

fwyzard commented Jul 25, 2017

cmsbuild commented Jul 25, 2017

fwyzard commented Jul 26, 2017

cmsbuild commented Jul 3, 2017 •

edited

Loading

cmsbuild commented Jul 13, 2017 •

edited

Loading