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

JanFSchulte · 2022-02-28T14:00:11Z

This PR improves the matching of L3 and L1 candidates in filters in the Muon HLT in two ways:

For L3/L1 matching, the L3 muon is now propagated to the second muon station, since L1 parameters are defined there. This improves matching efficiency for very low pT muons. This change has negligible impact on overall HLT performance. Total HLT rate is virtually unchange (0.04% increase). Timing is virtually unchange as well, despite the added propagation (black is without, red is with the propagation)
In HLTMuonL1TFilter 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.

…matching in HLTMuonL1TFilter

cmsbuild · 2022-02-28T14:09:02Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37086/28571

This PR adds an extra 32KB to repository

cmsbuild · 2022-02-28T14:09:24Z

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

Martin-Grunewald · 2022-02-28T14:27:58Z

please test

missirol · 2022-02-28T14:34:57Z

HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.h

@@ -45,6 +46,8 @@ class HLTMuonDimuonL3Filter : public HLTFilter {
                            const reco::RecoChargedCandidateRef&,
                            const reco::BeamSpot&,
                            const edm::ESHandle<MagneticField>&) const;
+
+  mutable PropagateToMuon prop_;


I wonder if the mutable attribute here might clash with the need to use this object inside a global module (which HLTFilter is). Maybe one step in the right direction is to use std::atomic. I can't judge with certainty, so I would kindly ask advise from @cms-sw/core-l2.

On a quick look PropagateToMuon::init() appears to set various member variables, and intended to be called on each event, so the use of mutable here indeed looks unsafe. The std::atomic won't help for many reasons, and any synchronization mechanism (like std::mutex) would be bad because the critical section is essentially the duration of the entire filter() method.

Here are some alternatives (in no particular order)

place PropagateToMuon object into an edm::StreamCache (or make the entire moduleedm::stream, but that might well be an overkill)

refactor PropagateToMuon into two classes such that

one class contains the current constructor and the init() function (as const), but instead of writing to member variables, it would create an object of an another class (below)

another class that contains the member variables currently being written to in init(), and all the extrapolate() const methods

move PropagateToMuon into EventSetup, in which case the ESProducer would take the role of the current constructor, and the ESProcucer::produce() + the constructor of PropagateToMuon would take the role of the current init() method

Thanks, @makortel . @JanFSchulte , I think this point will need to be addressed before moving forward with this PR.

Thanks for the feedback @makortel @missirol . Given that the impact on the HLT performance is minimal and we included this update mostly because it is technically the more correct way to perform the matching, I am not sure the effort is worth the gain. We might end up going with the just the fix to the L1 filter that is unaffected by this question and is an actual performance improvement.

missirol · 2022-02-28T15:31:23Z

HLTrigger/Muon/plugins/HLTMuonL1TFilter.cc

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


Suggested change

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

matchPrevL1 = true;

}

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

matchPrevL1 = true;

break;

}

}

Looks like the 0.1 qualifies as "magic number" here, so it would need to be replaced by a configuration parameter (held by a class member as in the suggested snippet). Also, is 0.1 the threshold for Delta-R or Delta-R^2 ?

I have made the 0.1 configurable in the factorized PR. In the way it's currently used, it is basically arbitrary since the parameters of the L1 candidates that need to be matched are identical.

cmsbuild · 2022-02-28T16:57:05Z

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-fdfc95/22714/summary.html
COMMIT: 0a1dd53
CMSSW: CMSSW_12_3_X_2022-02-28-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37086/22714/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

11634.0

----- Begin Fatal Exception 28-Feb-2022 17:08:51 CET-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 3 stream: 0
   [1] Running path 'HLT_Mu37_TkMu27_v5'
   [2] Calling method for module HLTMuonTrkL1TFilter/'hltL3fL1sMu16orMu25L1f0L2f25L3Filtered37'
Exception Message:
No data of type "Propagator" with label "SteppingHelixPropagatorAlong" in record "TrackingComponentsRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

11634.7

----- Begin Fatal Exception 28-Feb-2022 17:09:19 CET-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 3 stream: 0
   [1] Running path 'HLT_Mu37_TkMu27_v5'
   [2] Calling method for module HLTMuonTrkL1TFilter/'hltL3fL1sMu16orMu25L1f0L2f25L3Filtered37'
Exception Message:
No data of type "Propagator" with label "SteppingHelixPropagatorAlong" in record "TrackingComponentsRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

11634.914

----- Begin Fatal Exception 28-Feb-2022 17:10:24 CET-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 3 stream: 0
   [1] Running path 'HLT_Mu37_TkMu27_v5'
   [2] Calling method for module HLTMuonTrkL1TFilter/'hltL3fL1sMu16orMu25L1f0L2f25L3Filtered37'
Exception Message:
No data of type "Propagator" with label "SteppingHelixPropagatorAlong" in record "TrackingComponentsRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

Expand to see more relval errors ...

RelVals-INPUT

11608.011608.0_SingleMuPt100+2021+SingleMuPt100_Eta2p85_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA/step2_SingleMuPt100+2021+SingleMuPt100_Eta2p85_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA.log
11609.011609.0_SingleMuPt1000+2021+SingleMuPt1000_Eta2p85_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA/step2_SingleMuPt1000+2021+SingleMuPt1000_Eta2p85_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA.log
11630.011630.0_QCD_Pt_3000_3500_14TeV+2021+QCD_Pt_3000_3500_14TeV_TuneCUETP8M1_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA/step2_QCD_Pt_3000_3500_14TeV+2021+QCD_Pt_3000_3500_14TeV_TuneCUETP8M1_GenSimINPUT+Digi+RecoNano+HARVESTNano+ALCA.log

Expand to see more relval errors ...

AddOn Tests

hlt_mc_GRun

----- Begin Fatal Exception 28-Feb-2022 16:44:06 CET-----------------------
An exception of category 'NoProxyException' occurred while
   [0] Processing  Event run: 1 lumi: 1 event: 8 stream: 1
   [1] Running path 'HLT_Mu37_TkMu27_v5'
   [2] Calling method for module HLTMuonTrkL1TFilter/'hltL3fL1sMu16orMu25L1f0L2f25L3Filtered37'
Exception Message:
No data of type "Propagator" with label "SteppingHelixPropagatorAlong" in record "TrackingComponentsRecord"
 Please add an ESSource or ESProducer to your job which can deliver this data.
----- End Fatal Exception -------------------------------------------------

JanFSchulte · 2022-03-01T18:38:44Z

We have decided to factorize this PR and proceed with the more urgent, and much easier fix for the L1 candidate matching in #37110. Closing this PR for now and will follow up once we have solved the concerns for the muon propagation.

JanFSchulte added 2 commits February 28, 2022 14:31

add propagation to second muon station for L3/L1 matching and fix L1 …

58e2ff5

…matching in HLTMuonL1TFilter

code format and checkes

0a1dd53

cmsbuild added this to the CMSSW_12_3_X milestone Feb 28, 2022

cmsbuild added code-checks-pending hlt-pending orp-pending pending-signatures tests-pending labels Feb 28, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 28, 2022

cmsbuild added tests-started and removed tests-pending labels Feb 28, 2022

missirol reviewed Feb 28, 2022

View reviewed changes

cmsbuild added tests-rejected and removed tests-started labels Feb 28, 2022

JanFSchulte mentioned this pull request Mar 1, 2022

fix L1 candidate matching in HLTMuonL1TFilter #37110

Merged

JanFSchulte closed this Mar 1, 2022

JanFSchulte mentioned this pull request Mar 8, 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

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

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

JanFSchulte commented Feb 28, 2022

cmsbuild commented Feb 28, 2022

cmsbuild commented Feb 28, 2022

Martin-Grunewald commented Feb 28, 2022

missirol Feb 28, 2022

makortel Feb 28, 2022 •

edited

Loading

missirol Feb 28, 2022

JanFSchulte Feb 28, 2022

missirol Feb 28, 2022

JanFSchulte Mar 1, 2022

cmsbuild commented Feb 28, 2022

JanFSchulte commented Mar 1, 2022

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

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

Conversation

JanFSchulte commented Feb 28, 2022

cmsbuild commented Feb 28, 2022

cmsbuild commented Feb 28, 2022

Martin-Grunewald commented Feb 28, 2022

missirol Feb 28, 2022

Choose a reason for hiding this comment

makortel Feb 28, 2022 • edited Loading

Choose a reason for hiding this comment

missirol Feb 28, 2022

Choose a reason for hiding this comment

JanFSchulte Feb 28, 2022

Choose a reason for hiding this comment

missirol Feb 28, 2022

Choose a reason for hiding this comment

JanFSchulte Mar 1, 2022

Choose a reason for hiding this comment

cmsbuild commented Feb 28, 2022

RelVals

RelVals-INPUT

AddOn Tests

JanFSchulte commented Mar 1, 2022

makortel Feb 28, 2022 •

edited

Loading