fix L1 candidate matching in HLTMuonL1TFilter #37110

JanFSchulte · 2022-03-01T18:37:34Z

(split from #37086)

Fix to the L1 candidate matching in HLTMuonL1TFilter. In this filter a match between L1 muons before and after different filters is performed. Currently the code expects the muons that are compared to be in the same collection. However, there are use cases in the current menu where the matching is performed with a copy of the L1 muon collection and thus always fails. This causes an inefficiency for low pT muons that are reconstructed only by L1-seeded reconstruction steps. Impact is mostly negligible, showing tiny efficiency improvements for high-pT muon from the Z:

Effect gets more visible for lower pT muons, for example in the turnon of the double-muon trigger:

However, the effect is significant for very low pT leptons. For the muon Bs -> mumu trigger (HLT_DoubleMu4_3_Bs_v14) we find a ~20% improvement of HLT efficiency on a sample of signal events provided by BPH.

cmsbuild · 2022-03-01T18:44:08Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37110/28605

This PR adds an extra 16KB to repository

cmsbuild · 2022-03-01T18:44:31Z

A new Pull Request was created by @JanFSchulte (Jan-Frederik Schulte) for master.

It involves the following packages:

HLTrigger/Muon (hlt)

@cmsbuild, @missirol, @Martin-Grunewald can you please review it and eventually sign? Thanks.
@Fedespring, @HuguesBrun, @calderona, @silviodonato, @abbiendi, @jhgoh, @Martin-Grunewald, @missirol, @sscruz, @CeliaFernandez, @trocino, @cericeci this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

missirol · 2022-03-01T19:01:58Z

HLTrigger/Muon/plugins/HLTMuonL1TFilter.cc

+      bool matchPrevL1 = false;
+      int prevSize = prevMuons.size();
+      for (int it2 = 0; it2 < prevSize; it2++) {
+        if (deltaR2(muon->eta(), muon->phi(), prevMuons[it2]->eta(), prevMuons[it2]->phi()) < maxDR_)


General comment: there is some ambiguity in "deltaR vs deltaR2" (the parameter name suggests it's for DeltaR, while the actual cut is on DeltaR2). As done already here, the actual cut should be on DeltaR^2. My suggestion is
to mimic how this is handled here, reading the "maxDeltaR" value from the configuration, checking that it is positive, and then using the maxDeltaR2 value for the cut. Please also change maxDR to maxDeltaR to improve clarity.

Ok, I have implemented it according to the suggestion.

cmsbuild · 2022-03-01T19:39:48Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37110/28607

This PR adds an extra 16KB to repository

cmsbuild · 2022-03-01T19:40:06Z

Pull request #37110 was updated. @cmsbuild, @missirol, @Martin-Grunewald can you please check and sign again.

missirol

Here's the (short) review.

missirol · 2022-03-01T19:42:52Z

HLTrigger/Muon/plugins/HLTMuonL1TFilter.cc

-
+  //make sure cut parameter for candidate matching is
+  if (maxDR_ <= 0.) {
+    throw cms::Exception("HLTPFJetsMatchedToFilteredJetsProducerConfiguration")


Suggested change

throw cms::Exception("HLTPFJetsMatchedToFilteredJetsProducerConfiguration")

throw cms::Exception("HLTMuonL1TFilterConfiguration")

missirol · 2022-03-01T19:44:41Z

HLTrigger/Muon/plugins/HLTMuonL1TFilter.cc

@@ -38,7 +41,11 @@ HLTMuonL1TFilter::HLTMuonL1TFilter(const edm::ParameterSet& iConfig)
    //     }
    qualityBitMask_ |= 1 << selectQualitie;
  }
-
+  //make sure cut parameter for candidate matching is


Suggested change

//make sure cut parameter for candidate matching is

//make sure cut parameter for candidate matching is strictly positive

missirol · 2022-03-01T20:05:36Z

HLTrigger/Muon/plugins/HLTMuonL1TFilter.cc

+        if (deltaR2(muon->eta(), muon->phi(), prevMuons[it2]->eta(), prevMuons[it2]->phi()) < maxDR2_)
+          matchPrevL1 = true;
+        break;


Suggested change

if (deltaR2(muon->eta(), muon->phi(), prevMuons[it2]->eta(), prevMuons[it2]->phi()) < maxDR2_)

matchPrevL1 = true;

break;

if (deltaR2(muon->eta(), muon->phi(), prevMuons[it2]->eta(), prevMuons[it2]->phi()) < maxDR2_) {

matchPrevL1 = true;

break;

}

Careful with brackets :)

Yikes, I am really not awake enough today to code. Fix incoming...

@JanFSchulte , while you are at it, consider squashing into 1 commit (optional).

Sure, done.

cmsbuild · 2022-03-01T20:08:17Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37110/28610

This PR adds an extra 16KB to repository

cmsbuild · 2022-03-01T20:08:38Z

Pull request #37110 was updated. @cmsbuild, @missirol, @Martin-Grunewald can you please check and sign again.

missirol · 2022-03-01T20:13:57Z

type bugfix

cmsbuild · 2022-03-02T13:44:09Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37110/28628

This PR adds an extra 16KB to repository

cmsbuild · 2022-03-02T13:44:31Z

Pull request #37110 was updated. @cmsbuild, @missirol, @Martin-Grunewald can you please check and sign again.

Martin-Grunewald · 2022-03-02T13:50:12Z

please test

cmsbuild · 2022-03-02T17:54:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7c0076/22768/summary.html
COMMIT: e335344
CMSSW: CMSSW_12_3_X_2022-03-01-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37110/22768/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 4000857
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 4000833
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 204 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

missirol · 2022-03-02T19:33:47Z

+hlt

cmsbuild · 2022-03-02T19:34:06Z

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)

perrotta · 2022-03-02T20:36:18Z

+1

cmsbuild added this to the CMSSW_12_3_X milestone Mar 1, 2022

cmsbuild added code-checks-pending hlt-pending orp-pending pending-signatures tests-pending labels Mar 1, 2022

JanFSchulte mentioned this pull request Mar 1, 2022

Fixes for L3 and L1 trigger object matching in Muon HLT #37086

Closed

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 1, 2022

missirol reviewed Mar 1, 2022

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved labels Mar 1, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 1, 2022

missirol reviewed Mar 1, 2022

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved labels Mar 1, 2022

missirol reviewed Mar 1, 2022

View reviewed changes

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 1, 2022

cmsbuild added the bug-fix label Mar 1, 2022

fix L1 candidate matching in HLTMuonL1TFilter

2f7309a

JanFSchulte force-pushed the L1MatchFix branch from deafa87 to 2f7309a Compare March 1, 2022 20:17

cmsbuild added code-checks-pending hlt-pending pending-signatures tests-pending and removed tests-approved code-checks-approved labels Mar 2, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 2, 2022

cmsbuild added tests-started and removed tests-pending labels Mar 2, 2022

cmsbuild added tests-approved and removed tests-started labels Mar 2, 2022

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels Mar 2, 2022

cmsbuild added orp-approved and removed orp-pending labels Mar 2, 2022

cmsbuild merged commit 7e86ea4 into cms-sw:master Mar 2, 2022

This was referenced Mar 2, 2022

Migrate deprecated EDAnalyzers in CondTools/DT to the One version. #37124

Merged

Implementation of beam halo tagger algorithm for the endcap photons #36901

Merged

consolidate CalibTracker/SiStripQuality #37120

Merged

missirol mentioned this pull request Mar 15, 2022

Refactore PropagateToMuon and add propagation to L3 muon filters at HLT #37180

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix L1 candidate matching in HLTMuonL1TFilter #37110

fix L1 candidate matching in HLTMuonL1TFilter #37110

JanFSchulte commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

missirol Mar 1, 2022 •

edited

Loading

JanFSchulte Mar 1, 2022

cmsbuild commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

missirol left a comment

missirol Mar 1, 2022

missirol Mar 1, 2022

missirol Mar 1, 2022

JanFSchulte Mar 1, 2022

missirol Mar 1, 2022

JanFSchulte Mar 1, 2022

cmsbuild commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

missirol commented Mar 1, 2022

cmsbuild commented Mar 2, 2022

cmsbuild commented Mar 2, 2022

Martin-Grunewald commented Mar 2, 2022

cmsbuild commented Mar 2, 2022

missirol commented Mar 2, 2022

cmsbuild commented Mar 2, 2022

perrotta commented Mar 2, 2022

	throw cms::Exception("HLTPFJetsMatchedToFilteredJetsProducerConfiguration")
	throw cms::Exception("HLTMuonL1TFilterConfiguration")

	//make sure cut parameter for candidate matching is
	//make sure cut parameter for candidate matching is strictly positive

fix L1 candidate matching in HLTMuonL1TFilter #37110

fix L1 candidate matching in HLTMuonL1TFilter #37110

Conversation

JanFSchulte commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

missirol Mar 1, 2022 • edited Loading

Choose a reason for hiding this comment

JanFSchulte Mar 1, 2022

Choose a reason for hiding this comment

cmsbuild commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

missirol left a comment

Choose a reason for hiding this comment

missirol Mar 1, 2022

Choose a reason for hiding this comment

missirol Mar 1, 2022

Choose a reason for hiding this comment

missirol Mar 1, 2022

Choose a reason for hiding this comment

JanFSchulte Mar 1, 2022

Choose a reason for hiding this comment

missirol Mar 1, 2022

Choose a reason for hiding this comment

JanFSchulte Mar 1, 2022

Choose a reason for hiding this comment

cmsbuild commented Mar 1, 2022

cmsbuild commented Mar 1, 2022

missirol commented Mar 1, 2022

cmsbuild commented Mar 2, 2022

cmsbuild commented Mar 2, 2022

Martin-Grunewald commented Mar 2, 2022

cmsbuild commented Mar 2, 2022

Comparison Summary

missirol commented Mar 2, 2022

cmsbuild commented Mar 2, 2022

perrotta commented Mar 2, 2022

missirol Mar 1, 2022 •

edited

Loading