Add tau3mu HLT producer #19696

rmanzoni · 2017-07-12T07:15:15Z

Add new HLT producer that builds a reco::CompositeCandidate tau out of 3 muons.
Then it applies further selection, most notably tracker based isolation around the tau candidate.

cmsbuild · 2017-07-12T07:15:34Z

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

It involves the following packages:

HLTrigger/Muon

@Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks.
@battibass, @abbiendi, @jhgoh, @Martin-Grunewald, @calderona, @HuguesBrun, @trocino this is something you requested to watch as well.
@davidlange6 you are the release manager for this.

cms-bot commands are listed here

Martin-Grunewald · 2017-07-12T08:12:29Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+        // Create the 3-muon candidates
+        // loop over L3/Trk muons and create all combinations
+        for (unsigned int i = 0; i != AllMuCands->size(); ++i){
+            const reco::TrackRef &tk_i = (*AllMuCands)[i].track();


Use some modern-style loop constructs such as for (auto const & muon : AllMuCands) {...
for these three loops.

I think I prefer this style, as it is more legible.
auto obfuscates things, IMO, and I'm not sure that nested range-based loops would improve the legibility.

But let me try anyways and see if it ends up nicely enough.

I don't think you can use a range-based for loop here, since you use the index of the outer loop as the starting point of the inner loop, and so on.

It might be slightly more efficient using iterators, but the difference is probably negligible:

auto AllMuCands_end = AllMuCands->end(); for (auto i = AllMuCands->begin(); i != AllMuCands_end; ++i) { const reco::TrackRef &tk_i = i->track(); for (auto j = i+1; j != AllMuCands_end; ++j) { const reco::TrackRef &tk_j = j->track(); ...

and so on

ah yes, of course!

ok, understood.
So I give up my attempts at playing magic with ranges and move to iterators

Or just leave the indices here, if you find it more readable. The difference is just a multiplication and a sum.

Martin-Grunewald · 2017-07-12T08:13:51Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+            if (    itau->pt()   < TriMuonPtCut_  ) continue;
+            if (    itau->mass() < MinTriMuonMass_) continue;
+            if (    itau->mass() > MaxTriMuonMass_) continue;
+            if (abs(itau->eta()) > TriMuonEtaCut_ ) continue;


Use std::abs instead of just abs everywhere

indeed, abs will truncate to an integer !

yeah, pretty lame of me

Martin-Grunewald · 2017-07-12T08:14:41Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+                for (unsigned int k = j+1; k != AllMuCands->size(); ++k){
+                    const reco::TrackRef &tk_k = (*AllMuCands)[k].track();
+
+                    int passingPreviousFilter_1 = 0;


Could be calculated in the first loop

Martin-Grunewald · 2017-07-12T08:14:56Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+                    const reco::TrackRef &tk_k = (*AllMuCands)[k].track();
+
+                    int passingPreviousFilter_1 = 0;
+                    int passingPreviousFilter_2 = 0;


Could be calculated and kept in the 2nd loop

Martin-Grunewald · 2017-07-12T08:15:16Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+
+                    int passingPreviousFilter_1 = 0;
+                    int passingPreviousFilter_2 = 0;
+                    int passingPreviousFilter_3 = 0;


Only this would need to be calculated within the 3rd loop

Martin-Grunewald · 2017-07-12T08:16:01Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+                    if (((passingPreviousFilter_1 < 1) & (passingPreviousFilter_2 < 1)) ||
+                        ((passingPreviousFilter_1 < 1) & (passingPreviousFilter_3 < 1)) ||
+                        ((passingPreviousFilter_2 < 1) & (passingPreviousFilter_3 < 1))) continue;
+


IOW, the above should be caluclated less often in the corresponding loop for i/1, j/2 and k/3.

Martin-Grunewald · 2017-07-14T09:50:15Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+                    if (!( (passingPreviousFilter_1 & passingPreviousFilter_2 ) ||
+                           (passingPreviousFilter_1 & passingPreviousFilter_3 ) ||
+                           (passingPreviousFilter_2 & passingPreviousFilter_3 ) )) continue;
+


Actually, if the passingPreviousFilter_1/2/3 were all unsigned ints - either 0 or 1,
then the condition would be simply passingPreviousFilter_1+passingPreviousFilter_2+passingPreviousFilter_3<3

I tried it, but I found out that you can have the same muon appearing twice in the output of the previous filter, just because there is more than one combination that picks it up.

For example if the two good pairs are (1,2) and (1,3), you end up having 1 appearing twice and that would bump the sum up artificially

In light of the comment you made below, here the condition reduces to checking that
passingPreviousFilter_3 is true, correct?
wrong...

You mean the same muons appears twice in the collection?
Otherwise, the nested loops start at +1 index off the outer loop index, so the same
muon is not used twice.

IOW, even with (1,2) and (1,3), passingPreviousFilter_1 would be at most 1.

the same muon can appear more than once in PassedL3Muons, so if I loop on the entire PassedL3Muons collection for each muon of AllMuCands, even in the nested loop, that muon of AllMuCands can bump the counter several times

Martin-Grunewald · 2017-07-14T09:51:07Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+                    reco::TrackRef candTrkRef = (*imu)->get<reco::TrackRef>();
+                    if (reco::deltaR2(tk_j->momentum(), candTrkRef->momentum()) < (0.03*0.03)) passingPreviousFilter_2 = true;                
+                }
+                for (auto k = j+1; k != AllMuCands_end; ++k){


No need to execute 3rd loop if passingPreviousFilter_1 and passingPreviousFilter_2 are both FALSE (or 0).

Martin-Grunewald · 2017-07-14T09:52:01Z

HLTrigger/Muon/interface/HLTTriMuonIsolation.h

+            const reco::TrackRef &tk_i = i->track();
+            for (std::vector<reco::RecoChargedCandidateRef>::const_iterator imu = PassedL3Muons.begin(); imu != PassedL3Muons.end(); ++imu){
+                reco::TrackRef candTrkRef = (*imu)->get<reco::TrackRef>();
+                if (reco::deltaR2(tk_i->momentum(), candTrkRef->momentum()) < (0.03*0.03)) passingPreviousFilter_1 = true;


0.03 is a magic number - better make it a configurable plugin parameter and of course add it to fillDescriptions()

Martin-Grunewald · 2017-07-15T13:36:58Z

please test

cmsbuild · 2017-07-15T13:37:12Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21503/console Started: 2017/07/15 15:37

cmsbuild · 2017-07-15T14:48:30Z

+1
Tested at: a3a2fd1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19696/21503/summary.html

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

cmsbuild · 2017-07-15T14:48:33Z

Comparison job queued.

cmsbuild · 2017-07-17T15:26:49Z

Pull request #19696 was updated. @Martin-Grunewald, @silviodonato, @cmsbuild, @fwyzard, @davidlange6 can you please check and sign again.

Martin-Grunewald · 2017-07-18T06:29:00Z

please test

cmsbuild · 2017-07-18T06:29:18Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/21548/console Started: 2017/07/18 08:30

cmsbuild · 2017-07-18T07:51:59Z

+1
Tested at: dc4fe75
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-19696/21548/summary.html

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

cmsbuild · 2017-07-18T07:52:02Z

Comparison job queued.

Martin-Grunewald · 2017-07-18T07:56:03Z

+1

cmsbuild · 2017-07-18T07:56:19Z

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 requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar

cmsbuild · 2017-07-18T09:14:00Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 2025409
DQMHistoTests: Total failures: 108
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2025135
DQMHistoTests: Total skipped: 166
DQMHistoTests: Total Missing objects: 0
Checked 93 log files, 14 edm output root files, 23 DQM output files

davidlange6 · 2017-07-18T11:30:34Z

+1

add tau3mu HLT producer

e0b3aa8

cmsbuild added this to the CMSSW_9_3_X milestone Jul 12, 2017

cmsbuild added comparison-pending hlt-pending orp-pending pending-signatures tests-pending labels Jul 12, 2017

rmanzoni mentioned this pull request Jul 12, 2017

Add tau3mu HLT producer #19697

Merged

Martin-Grunewald reviewed Jul 12, 2017

View reviewed changes

improve loop logic, anticipate dZ cut

3a1a02a

Martin-Grunewald reviewed Jul 14, 2017

View reviewed changes

rmanzoni added 3 commits July 14, 2017 12:09

spare a loop if you can

049a2d3

remove unneeded include from boost lib

32439c5

add option to require collimated muons

a3a2fd1

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

cmsbuild added tests-approved and removed tests-started labels Jul 15, 2017

cmsbuild added comparison-pending hlt-pending pending-signatures tests-pending and removed comparison-available fully-signed hlt-approved tests-approved labels Jul 17, 2017

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

cmsbuild added tests-approved and removed tests-started labels Jul 18, 2017

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels Jul 18, 2017

cmsbuild added comparison-available and removed comparison-pending labels Jul 18, 2017

cmsbuild added orp-approved and removed orp-pending labels Jul 18, 2017

cmsbuild merged commit 6d67a00 into cms-sw:master Jul 18, 2017

Add tau3mu HLT producer #19696

Add tau3mu HLT producer #19696

Conversation

rmanzoni commented Jul 12, 2017

cmsbuild commented Jul 12, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmanzoni Jul 14, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rmanzoni Jul 14, 2017 • edited

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

Martin-Grunewald commented Jul 15, 2017

cmsbuild commented Jul 15, 2017 • edited

cmsbuild commented Jul 15, 2017

cmsbuild commented Jul 15, 2017

cmsbuild commented Jul 17, 2017

Martin-Grunewald commented Jul 18, 2017

cmsbuild commented Jul 18, 2017 • edited

cmsbuild commented Jul 18, 2017

cmsbuild commented Jul 18, 2017

Martin-Grunewald commented Jul 18, 2017

cmsbuild commented Jul 18, 2017

cmsbuild commented Jul 18, 2017

davidlange6 commented Jul 18, 2017

rmanzoni Jul 14, 2017 •

edited

rmanzoni Jul 14, 2017 •

edited

cmsbuild commented Jul 15, 2017 •

edited

cmsbuild commented Jul 18, 2017 •

edited