Added new HLTfilter to select L3 and L2 muons #36239

alexsr-98 · 2021-11-24T18:08:50Z

PR description:

This PR adds a HLTfilter (HLTMuonL3andL2PreFilter.cc) designed to filter L2 and L3 muons. If the L3 exists it is saved together with all the L2 muons which did not match with any L3.
There is the possibility to cut on different variables of the L2 and L3 muons independently.
There are two different parameters to control the minimum number of L2 and L3 muons that you want to have in order to trigger the event.
This development has been presented in several Muon POG meetings: presentation.

PR validation:

Passed the "scram b runtests" test.

cmsbuild · 2021-11-24T18:16:38Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/26872

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/26872/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/26872/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

missirol · 2021-11-24T20:25:28Z

@alexsr-98

This PR can't proceed without first passing code checks. Please follow
https://cms-sw.github.io/PRWorkflow.html

cmsbuild · 2021-11-25T11:18:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/26895

This PR adds an extra 20KB to repository

cmsbuild · 2021-11-25T11:19:03Z

A new Pull Request was created by @alexsr-98 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

A first round of comments. I have not completed yet the review of HLTMuonL3orL2PreFilter::hltFilter, but these can be addressed in the meantime.

HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc

missirol · 2021-11-25T15:06:54Z

HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc

+    if (L2tk->normalizedChi2() > max_NormalizedChi2_L2_)
+      FailedL2 = true;
+
+    if (recoBeamSpotHandle.isValid()) {


Suggested change

if (recoBeamSpotHandle.isValid()) {

if (useBeamspot_) {

if (not recoBeamSpotHandle.isValid()) { throw cms::Exception("") << ""; }

(the suggestion is pseudo-code..)

If the beamspot handle is invalid (e.g. due to a typo in the InputTag), certain cuts/selections are silently skipped. It would be safer to add a boolean useBeamspot in the PSet, and to throw an exception if useBeamspot==true and recoBeamSpotHandle.isValid() == false.

I have added the exception but I think that it is not necessary to include another parameter to use the beam spot. If you want to use the cuts that are implemented under this bracket you can change for example the parameter max_Dz_. The beam spot is used only to compute Dz (the impact parameter in the z direction).

HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc

Martin-Grunewald · 2021-11-25T16:32:16Z

Sorry, but 'the code compiles' is insufficient as validation, please run a trigger path exercising this filter and check(!) that it performs as intended in terms of recording L2/L3 candidates, and that this is done (technical) correctly by adding to your HLT path with the new plugin also the HLTriggerFinalPath which does the packing up (and will complain with error messages in case of miscodings).

Or in other words: first develop your trigger paths using this code in your developer area, present to the TSG efficiencies, rates and timing, and then submit code (CMSSW PR) and JIRA ticket (request for integration of a/the new path[s]).

folguera · 2021-11-26T09:19:10Z

@Martin-Grunewald we are in the process of developing a new reconstruction sequence for long-lived particles, and this filter has been fully validated in our development area with a mockup path that we are using for validation of efficiencies, rates and timing.The final HLT that we will propose will come after.

The motivation for this PR is that we need to share the reconstruction sequence with other groups and we did plan to use confdb for this, and if I'm not mistaken, we would need this filter to be merged in order to be picked by confDB.

Martin-Grunewald · 2021-11-26T15:11:06Z

OK, that's good. Still, more checking is needed as per my code comment above - you should have seen error messages arising from one of the modules in the HLTriggerFinalPath...

cmsbuild · 2021-11-27T16:10:37Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/26941

ERROR: Build errors found during clang-tidy run.

         reco::BeamSpot
--
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:115:40: error: use of undeclared identifier 'L2mucands'; did you mean 'mucands'? [clang-diagnostic-error]
  for (unsigned int iMuL2 = 0; iMuL2 < L2mucands->size(); iMuL2++) {
                                       ^~~~~~~~~
                                       mucands
--
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:116:5: error: unknown type name 'RecoChargedCandidateRef'; did you mean 'reco::RecoChargedCandidateRef'? [clang-diagnostic-error]
    RecoChargedCandidateRef L2cand(L2mucands, iMuL2);
    ^~~~~~~~~~~~~~~~~~~~~~~
    reco::RecoChargedCandidateRef
--
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:116:36: error: use of undeclared identifier 'L2mucands'; did you mean 'mucands'? [clang-diagnostic-error]
    RecoChargedCandidateRef L2cand(L2mucands, iMuL2);
                                   ^~~~~~~~~
                                   mucands
--
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:126:5: error: unknown type name 'TrackRef'; did you mean 'reco::TrackRef'? [clang-diagnostic-error]
    TrackRef L2tk = L2cand->track();
    ^~~~~~~~
    reco::TrackRef
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

cmsbuild · 2021-11-27T16:23:51Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/26942

ERROR: Build errors found during clang-tidy run.

HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:12:1: error: use of undeclared identifier 'HLTMuonL3orL2PreFilter' [clang-diagnostic-error]
HLTMuonL3orL2PreFilter::HLTMuonL3orL2PreFilter(const edm::ParameterSet& iConfig)
^
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:46:6: error: use of undeclared identifier 'HLTMuonL3orL2PreFilter' [clang-diagnostic-error]
void HLTMuonL3orL2PreFilter::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
     ^
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:78:6: error: use of undeclared identifier 'HLTMuonL3orL2PreFilter' [clang-diagnostic-error]
bool HLTMuonL3orL2PreFilter::hltFilter(edm::Event& iEvent,
     ^
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:299:6: error: use of undeclared identifier 'HLTMuonL3orL2PreFilter' [clang-diagnostic-error]
bool HLTMuonL3orL2PreFilter::triggeredByPreviousLevel(const reco::RecoChargedCandidateRef& candref,
     ^
HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc:304:19: error: use of undeclared identifier 'HLTMuonL3orL2PreFilter' [clang-diagnostic-error]
DEFINE_FWK_MODULE(HLTMuonL3orL2PreFilter);
                  ^
Suppressed 1326 warnings (1325 in non-user code, 1 with check filters).
--
gmake: *** [config/SCRAM/GMake/Makefile.coderules:129: code-checks] Error 2
gmake: *** [There are compilation/build errors. Please see the detail log above.] Error 2

cmsbuild · 2022-02-08T09:34:20Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/28212

This PR adds an extra 20KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/28212/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/28212/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

Applied patch for code quality checks Apply suggestions from code review Applied some suggestions from code review. Co-authored-by: Marino Missiroli <m.missiroli@cern.ch> Apply suggestions from code review Applied more suggestions from code review Co-authored-by: Marino Missiroli <m.missiroli@cern.ch> Adressed first set of comments of the PR and added a customizer to implement the filter Applied patch for code quality checks Apply suggestions from code review Applied some suggestions from code review Co-authored-by: Marino Missiroli <m.missiroli@cern.ch> Applied suggestions from code review Applied code checks Applied comments from the PR Applied comment Applied code checks Apply suggestions from code review Implemented some comments from PR review. Co-authored-by: Marino Missiroli <m.missiroli@cern.ch> Implemented comments from PR review. Implemented code checks and removed customizer file Implemented comments from PR review Implemented code checks Reorganized the code to make it clearer and removed repeated code Applied code checks Implemented comments from PR review Applied code checks Small fix in the fillDescriptions function Applied comments from PR Applied code checks Renamed plugin Applied code checks

cmsbuild · 2022-02-08T09:49:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36239/28213

This PR adds an extra 20KB to repository

cmsbuild · 2022-02-08T09:50:06Z

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

missirol · 2022-02-08T09:54:31Z

please test

@alexsr-98 , the implementation looks okay to me, thanks.

The PR tests here won't exercise this new plugin (other than compiling it), so I assume you verified in your HLT studies that the latest version of the plugin produces the intended physics results (if not, please do verify this).

Also, since the plugin content has slightly changed during the review ("or" -> "and"), please update the PR title and description accordingly if need be.

cmsbuild · 2022-02-08T13:46:05Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-75a98a/22282/summary.html
COMMIT: 00be3e9
CMSSW: CMSSW_12_3_X_2022-02-07-2300/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/36239/22282/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: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 46
DQMHistoTests: Total histograms compared: 3765080
DQMHistoTests: Total failures: 7
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3765050
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 45 files compared)
DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
Checked 193 log files, 42 edm output root files, 46 DQM output files
TriggerResults: no differences found

alexsr-98 · 2022-02-08T14:41:16Z

please test

@alexsr-98 , the implementation looks okay to me, thanks.

The PR tests here won't exercise this new plugin (other than compiling it), so I assume you verified in your HLT studies that the latest version of the plugin produces the intended physics results (if not, please do verify this).

Also, since the plugin content has slightly changed during the review ("or" -> "and"), please update the PR title and description accordingly if need be.

Yes, I've checking it during the revision and it behaves as expected.

missirol · 2022-02-08T14:53:40Z

+hlt

addition of a new HLTFilter, not presently used in any wf or HLT menu

cmsbuild · 2022-02-08T14:54:00Z

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)

qliphy · 2022-02-09T01:44:19Z

+1

cmsbuild added this to the CMSSW_12_2_X milestone Nov 24, 2021

cmsbuild added code-checks-pending hlt-pending orp-pending pending-signatures tests-pending labels Nov 24, 2021

cmsbuild added code-checks-rejected and removed code-checks-pending labels Nov 24, 2021

cmsbuild added code-checks-pending and removed code-checks-rejected labels Nov 25, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Nov 25, 2021

missirol reviewed Nov 25, 2021

View reviewed changes

Martin-Grunewald reviewed Nov 25, 2021

View reviewed changes

HLTrigger/Muon/plugins/HLTMuonL3orL2PreFilter.cc Outdated Show resolved Hide resolved

cmsbuild added code-checks-pending and removed code-checks-approved labels Nov 27, 2021

cmsbuild added code-checks-rejected code-checks-pending and removed code-checks-pending code-checks-rejected labels Nov 27, 2021

cmsbuild removed the code-checks-pending label Nov 27, 2021

cmsbuild added the code-checks-pending label Feb 8, 2022

cmsbuild added code-checks-rejected and removed code-checks-pending labels Feb 8, 2022

alexsr-98 force-pushed the new-hlt-path-GblDisplaced branch from bbea25b to 00be3e9 Compare February 8, 2022 09:41

cmsbuild added code-checks-pending and removed code-checks-rejected labels Feb 8, 2022

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

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

alexsr-98 changed the title ~~Added new HLTfilter to select L3 or L2 muons~~ Added new HLTfilter to select L3 and L2 muons Feb 8, 2022

cmsbuild added tests-approved and removed tests-started labels Feb 8, 2022

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels Feb 8, 2022

cmsbuild added orp-approved and removed orp-pending labels Feb 9, 2022

cmsbuild merged commit e984ded into cms-sw:master Feb 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new HLTfilter to select L3 and L2 muons #36239

Added new HLTfilter to select L3 and L2 muons #36239

alexsr-98 commented Nov 24, 2021 •

edited

cmsbuild commented Nov 24, 2021

missirol commented Nov 24, 2021 •

edited

cmsbuild commented Nov 25, 2021

cmsbuild commented Nov 25, 2021

missirol left a comment

missirol Nov 25, 2021

alexsr-98 Dec 12, 2021

Martin-Grunewald commented Nov 25, 2021 •

edited

folguera commented Nov 26, 2021

Martin-Grunewald commented Nov 26, 2021

cmsbuild commented Nov 27, 2021

cmsbuild commented Nov 27, 2021

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

missirol commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

alexsr-98 commented Feb 8, 2022

missirol commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

qliphy commented Feb 9, 2022

	if (recoBeamSpotHandle.isValid()) {
	if (useBeamspot_) {
	if (not recoBeamSpotHandle.isValid()) { throw cms::Exception("") << ""; }

Added new HLTfilter to select L3 and L2 muons #36239

Added new HLTfilter to select L3 and L2 muons #36239

Conversation

alexsr-98 commented Nov 24, 2021 • edited

PR description:

PR validation:

cmsbuild commented Nov 24, 2021

missirol commented Nov 24, 2021 • edited

cmsbuild commented Nov 25, 2021

cmsbuild commented Nov 25, 2021

missirol left a comment

Choose a reason for hiding this comment

missirol Nov 25, 2021

Choose a reason for hiding this comment

alexsr-98 Dec 12, 2021

Choose a reason for hiding this comment

Martin-Grunewald commented Nov 25, 2021 • edited

folguera commented Nov 26, 2021

Martin-Grunewald commented Nov 26, 2021

cmsbuild commented Nov 27, 2021

cmsbuild commented Nov 27, 2021

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

missirol commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

Comparison Summary

alexsr-98 commented Feb 8, 2022

missirol commented Feb 8, 2022

cmsbuild commented Feb 8, 2022

qliphy commented Feb 9, 2022

alexsr-98 commented Nov 24, 2021 •

edited

missirol commented Nov 24, 2021 •

edited

Martin-Grunewald commented Nov 25, 2021 •

edited