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

[NGT] Implement new seeding module for HLT Standalone Muon seeding and streamline HLT L3 Tracker Muon reconstruction #46897

Merged
merged 6 commits into from
Dec 20, 2024

Conversation

Parsifal-2045
Copy link
Contributor

PR description:

This PR implements a new Standalone (L2) Muon seeding strategy that fully utilizes the L1 Tracker Muons (L1TkMu) information.
L3 Tracker Muon reconstruction is also streamlined, also taking advantage of the improved L1TkMu, to reduce redundancy.
The full scope of the changes was described in the HLT Upgrade meeting on 8/10/2024, while the details on the integration were presented in the meeting on 3/12/2024.

The new L2 seeder directly matches the stubs used to produce each L1TkMu with segments in DTs/CSCs. All matched segments are assigned to the seed, together with the pT measurement from the L1TkMu. This approach produces a single collection of seeds.
On the other hand, the current implementation produces two collections of seeds (from L1TkMu and "offline") that require matching and produces roughly a factor of 5 more seeds in ZMM simulated events with 200 PU.
The changes implemented in the seeding module results in a significant improvement in timing (about 42%) with mostly compatible efficiency.

The streamlining of the L3 reconstruction hinges on the idea of performing either the Inside-Out or the Outside-In L3 Tracker Muon reconstruction first, to then execute a second pass only for candidates that require it (i.e. either they were not reconstructed in the first pass or the quality of the track was not good enough). This approach starts from either reconstruction path (flexible in case of future changes or detectors' requirements) to produce a filtered collection of L3 tracks: the filter uses some of the same criteria used in the HLT Muon ID step (e.g. number of hits in the pixel, number of hits in the tracker, normalized chi2, ...). The filtered L3 Tracks are geometrically matched to either L1TkMu (Outside-In first) or Standalone Muons (Inside-Out first):

  • if the match is successful, the track is considered good enough and passes to the next phase;
  • if the match is unsuccessful, the lower-level object will be reused to seed the second reconstruction path.
    From the current results, the Inside-Out first approach seems to perform better, with more than 80% of the events in a ZMM sample at 200 PU requiring a single reconstruction path, to be compared with about 20% for the Outside-In first approach.
    In any case, the streamlined L3 reconstruction reduces the fakerate in the merged (L3 IO + L3 OI) tracks collection as well as timing, while maintaining compatible physics perfomance until and including the final HLT Muon ID objects. All results, validation plots and ntuples are available here

The changes have been implemented using two new process modifiers:

  • phase2L2AndL3Muons implements the new Standalone Muon seeding and executes the L3 Tracker Muon reconstruction Inside-Out first;
  • phase2L3MuonsOIFirst to be used on top of the previous one, swaps the L3 Tracker Muon reconstruction from Inside-Out first to Outside-In first

These process modifiers have been included in a set of workflows targeting SingleMu, TTbar, and ZMM samples:

  • .777 includes the phase2L2AndL3Muons
  • .778 includes the phase2L2AndL3Muons and the phase2L3MuonsOIFirst process modifiers

PR validation:

Validation builds on top of #46860 introducing specific validation paths for the new objects (e.g. filtered collections, objects to reuse). Physics performance mostly compatible with the current implementation. Tested on CMSSW_14_2_0_pre3 RelVals (TTbar and ZMM) (not pre4 due to those being limited to 35 PU) as well as private production with workflows 29834(.0, .777, .778) (TTbar) and 29850 (.0, .777, .778) (ZMM).
Please test on Phase 2 workflows using also .777 and .778 modifiers

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2024

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46897/42945

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 9, 2024

A new Pull Request was created by @Parsifal-2045 for master.

It involves the following packages:

  • Configuration/EventContent (operations)
  • Configuration/ProcessModifiers (operations)
  • Configuration/PyReleaseValidation (pdmv, upgrade)
  • DataFormats/MuonSeed (reconstruction)
  • HLTrigger/Configuration (hlt)
  • RecoMuon/L2MuonSeedGenerator (hlt, reconstruction)
  • RecoMuon/L3TrackFinder (hlt, reconstruction)
  • Validation/RecoMuon (dqm)

@AdrianoDee, @Martin-Grunewald, @Moanwar, @antoniovagnerini, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @jfernan2, @mandrenguyen, @miquork, @mmusich, @rappoccio, @rseidita, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks.
@CeliaFernandez, @Fedespring, @HuguesBrun, @Martin-Grunewald, @SohamBhattacharya, @VourMa, @abbiendi, @andrea21z, @bellan, @calderona, @cericeci, @fabiocos, @jhgoh, @makortel, @missirol, @mmusich, @rociovilar, @rovere, @silviodonato, @slomeo, @trocino this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@mmusich
Copy link
Contributor

mmusich commented Dec 9, 2024

test parameters:

  • workflow_opts= -w upgrade
  • workflow = 29834.0, 29834.777, 29834.778, 29850.0, 29850.777, 29850.778
  • enable = hlt_p2_integration, hlt_p2_timing

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

These are just cosmetic comments from a quick transversal look.
I'll go through the dense C++ changes in the coming days.

A general request for clarification.
What is the plan to re-absorb phase2L3MuonsOIFirst and phase2L2AndL3Muons process modifiers in the mainstream configuration?

The question is not really for the PR proponent but I have a feeling we're diverging with the options for the phase-2 menu. @rovere @VourMa

RecoMuon/L2MuonSeedGenerator/src/Phase2L2MuonSeedCreator.h Outdated Show resolved Hide resolved
mat[1][1] = 0.05 * 0.05; // sigma^2(lambda)
mat[2][2] = 0.2 * 0.2; // sigma^2(phi)
mat[3][3] = 20. * 20.; // sigma^2(x_transverse)
mat[4][4] = 20. * 20.; // sigma^2(y_transverse)
Copy link
Contributor

Choose a reason for hiding this comment

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

where are all these numbers coming from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The propagation to the various muon stations have remained basically unchanged from the current L2 MuonSeedGenerator (seeded by L1Tk Muons)

Copy link
Contributor

Choose a reason for hiding this comment

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

should we consider storing these numbers in a single place for both modules? or is the old seed generator going to be discontinued?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The aim is for this new module to replace the current "legacy" seeding strategy. However, it's currently under a process modifier as a soft introduction while some slight inefficiencies are ironed out and its performance is assessed thoroughly. In any case, in the end, there should be a single Standalone Muon seed from L1Tk Muons generator

continue;
}
float eta = l1TkMuRef->phEta();
float theta = 2 * std::atan(std::exp(-eta));
Copy link
Contributor

Choose a reason for hiding this comment

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

this pattern together with the usage of ThreeVector from CLHEP smells.
What about something possibly more efficient reducing the usage of trig functions along the lines of 6272d52 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The latest commit should address this. It's still computing theta (only once per L1TkMu) since it's used for the stub-segment matching, but everything has been refactored to result in fewer overall "taxing" computations

- Takes full advantage of the Phase 2 L1 Tracker Muon information
- Direct match between L1 stubs and segments in DTs/CSCs
- Extract stubs from L1Tk Muon, only checks segments in the same chamber as the stub to match
- Match based on dphi, dtheta and number of hits in the segment
- In the barrel, if no match is found in a given station, a rough extrapolation is attempted from the closest station with a match
- At most one seed per L1Tk Muon produced in a single seeding step, with the possibility to extend to exotic signatures
- execute either Inside-Out or Outside-In reconstruction first, resorting to the second pass only for candidates that require it
- Tracks filtered by quality using some of the same criteria as HLT Muon ID
- L1/L2 objects matched with bad L3 tracks are reused to seed the second reconstruction step
- Changes implemented via 2 procModifiers for IO first (default) and OI first reconstruction
- Implemented changes in SingleMu, TTbar, and ZMM workflows with .777 and .778 subfixes for IO and OI first reconstruction, respectively
- Change 2026 to Run4 in Phase 2 workflows
@cmsbuild
Copy link
Contributor

+1

Size: This PR adds an extra 28KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-56c0a9/43495/summary.html
COMMIT: 93099c0
CMSSW: CMSSW_15_0_X_2024-12-16-2300/el8_amd64_gcc12
Additional Tests: HLT_P2_INTEGRATION,HLT_P2_TIMING
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/46897/43495/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 2 lines from the logs
  • ROOTFileChecks: Some differences in event products or their sizes found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 46
  • DQMHistoTests: Total histograms compared: 3510017
  • DQMHistoTests: Total failures: 416
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3509581
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 45 files compared)
  • Checked 202 log files, 172 edm output root files, 46 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented Dec 17, 2024

+hlt

@jfernan2
Copy link
Contributor

+1

@antoniovagnerini
Copy link

+dqm

@AdrianoDee
Copy link
Contributor

+pdmv

@mmusich
Copy link
Contributor

mmusich commented Dec 19, 2024

@cms-sw/l1-l2 can you please take a look to the simple change in the list of keep statements of L1Trigger/Configuration/python/L1Trigger_EventContent_cff.py ?
Given the amount of signatures involved in this PR, I'd like to avoid running into conflict.

@aloeliger
Copy link
Contributor

+l1

Looks fine from my end, thanks for catching the wrong "keep" statement because of the name change.

@cmsbuild
Copy link
Contributor

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. @mandrenguyen, @sextonkennedy, @rappoccio, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 6c30957 into cms-sw:master Dec 20, 2024
14 checks passed
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.