Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for invert mass window cut and Delta R for HLTMuonDimuonL3Filter #36250

Closed
wants to merge 31 commits into from

Conversation

arsahasransu
Copy link
Contributor

PR description:

  • This PR is a development towards CMS Run3 HLT where we trigger events with soft muons in them. To get a reasonable rate, new kinematic constraints on the dimuon events need to be placed at the HLT.

  • Added options to make a selection on the minimum angle required between L3 muons in the trigger

  • Added option to invert the mass window cut on the invariant mass of L3 muons

  • This is based on the presentation at https://indico.cern.ch/event/1090283/#33-level-3-paths-for-singlet-t

  • Sam James Harper noticed that error management in the module can be improved.

  • It is ideal to throw an exception at the constructor level when that error renders the module of no use. This enable catching the error in the build phase instead of having to wait for a message from cmsRun.

PR validation:

  • Standard set of checks performed at https://cms-sw.github.io/PRWorkflow.html were done and passed.
  • A trigger was made with the suggested changes. We observed expected behavior from this trigger of interest.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36250/26903

  • This PR adds an extra 24KB to repository

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

@missirol
Copy link
Contributor

@arsahasransu

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

Also, once you have fixed those, please squash all commits into 1 for clarity.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36250/26909

  • This PR adds an extra 24KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @arsahasransu (A.R.Sahasransu) 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

Copy link
Contributor

@missirol missirol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the first round of comments.

  • I don't see the necessity to introduce the invertDiMuonMassSelection parameter; couldn't the same be obtained through a proper configuration of the existing plugin? e.g.
   MinPtPair = cms.vdouble( 0., 0. ),
   MaxPtPair = cms.vdouble( 1.0E125, 1.0E125 ),
   MinPtMax = cms.vdouble( 0.0, 0.0 ),
   MinPtMin = cms.vdouble( 0.0, 0.0 ),
   MaxPtMin = cms.vdouble( 1.0E125, 1.0E125 ),
   MinInvMass = cms.vdouble( -1.0, 98. ),
   MaxInvMass = cms.vdouble( 84., 1.0E125 ),
  • the DeltaR cut should be disabled by default, and converted internally to a cut on DeltaR^2 for better computational performance.

HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.h Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.h Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
HLTrigger/Muon/plugins/HLTMuonDimuonL3Filter.cc Outdated Show resolved Hide resolved
@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36250/26915

  • 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)

@arsahasransu
Copy link
Contributor Author

I made a major mistake while trying to squash all the changes into a single commit. So I have closed this PR. I will make a fresh PR soon.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36250/26935

  • This PR adds an extra 72KB to repository

  • Found files with invalid states:

    • RecoLocalCalo/HcalRecAlgos/python/mahiDebugger_cfi.py:

@cmsbuild
Copy link
Contributor

Pull request #36250 was updated. @Martin-Grunewald, @civanch, @Dr15Jones, @makortel, @cvuosalo, @emanueleusai, @ianna, @mdhildreth, @cmsbuild, @missirol, @jfernan2, @ahmad3213, @slava77, @jpata, @pmandrik, @pbo0, @rvenditti can you please check and sign again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants