Skip to content

Add GEMs to FastSim - #51810

Merged
cmsbuild merged 9 commits into
cms-sw:masterfrom
kpedro88:fastSim_GEM_17_0_0_pre2
Sep 12, 2026
Merged

Add GEMs to FastSim#51810
cmsbuild merged 9 commits into
cms-sw:masterfrom
kpedro88:fastSim_GEM_17_0_0_pre2

Conversation

@kpedro88

@kpedro88 kpedro88 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

PR description:

This PR adds the simulation of GEMs to FastSim for relevant Run 2/3 scenarios. Thanks to @yeckang for doing the hard work of actually integrating the GEM geometry and validating the SimHit physics output. My contributions were technical (Eras, code improvements).

The GEM simulation is disabled for pre-2017 and post-Run3 workflows. No GEM chambers were installed before 2017, so the geometry is not available then. The Run4 GEM simulation will come in a separate PR.

The energy loss value is now computed and stored for all muon hits (needed by the GEM digitizer).

The per-year separate FastSim Eras (which existed to remove GEM from the corresponding per-year FullSim Eras) are no longer needed and are therefore removed. (#49111)

I took the opportunity to modernize the muon SimHit production code (similar to what was done for other parts of FastSim in #49583):

  • member variable names end in underscore
  • use range-based loops
  • capture temporary return values by const ref to extend lifetimes without copying
  • avoid repeated calculations (e.g. squaring numbers)
  • use named constants instead of magic numbers
  • use std::abs instead of fabs

Unfortunately, the design of MuonServiceProxy seems to preclude moving MuonSimHitProducer to be a global producer, since it both uses consumesCollector and then requires a non-const update() function to be called during beginRun. This class is indeed never used with a global module anywhere in CMSSW. It could be redesigned, but that is beyond the scope of this PR / the FastSim group.

A minor bug in the HGCal digi aliases for the Run4 workflow was incidentally noticed and fixed.

PR validation:

Ran the following workflows:

  • 135.1: FastSim Run 2, pre-GEM. Runs successfully, no GEM hit collection in output.
  • 16434.0: FastSim Run 3, with GEM. Runs successfully, GEM hits appear and have equivalent physics performance to other muon subdetectors (see internal validation at the FastSim meeting).
  • 36434.0: FastSim Run 4, currently still without GEM. Runs successfully, no GEM hit collection in output. (As mentioned, the Run 4 workflow will be updated separately.)

For the modernization commit in particular, comparing the SIM step alone with and without the changes, the event throughput increases from ~4.5 evt/s to ~6 evt/s (single threaded). I also confirmed that the muon SimHit collection output values are unchanged by these technical improvements.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

This will be backported to 17_0_X for future Run 3 MC production.

@cmsbuild

cmsbuild commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

cms-bot internal usage

@kpedro88
kpedro88 changed the base branch from CMSSW_17_0_X to master September 2, 2026 20:20
@cmsbuild

cmsbuild commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-51810/50804

@cmsbuild

cmsbuild commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

A new Pull Request was created by @kpedro88 for master.

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/PyReleaseValidation (pdmv)
  • Configuration/StandardSequences (operations)
  • FastSimulation/Configuration (fastsim)
  • FastSimulation/MuonSimHitProducer (fastsim)
  • SimGeneral/MixingModule (simulation)
  • SimGeneral/PreMixingModule (simulation)
  • SimMuon/GEMDigitizer (simulation)
  • SimMuon/MCTruth (simulation)
  • Validation/MuonCSCDigis (dqm)
  • Validation/MuonHits (dqm)

@AdrianoDee, @DickyChant, @antoniovagnerini, @civanch, @cmsbuild, @ctarricone, @davidlange6, @fabiocos, @ftenchini, @gabrielmscampos, @kfjack, @kpedro88, @mandrenguyen, @mdhildreth, @miquork, @rseidita, @sroychow, @ssekmen can you please review it and eventually sign? Thanks.
@24LopezR, @CeliaFernandez, @Fedespring, @GiacomoSguazzoni, @HuguesBrun, @Martin-Grunewald, @VinInn, @VourMa, @abbiendi, @cericeci, @dgulhan, @elusian, @fabiocos, @felicepantaleo, @fsimone91, @giovanni-mocellin, @jhgoh, @jshlee, @llunerti, @makortel, @martinamalberti, @matt-komm, @mmasciov, @mmusich, @mtosi, @rovere, @sameasy, @sbein, @slomeo, @trocino, @watson-ij this is something you requested to watch as well.
@ftenchini, @mandrenguyen, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

@kpedro88

kpedro88 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

test parameters:
workflows = 36434.0

@kpedro88

kpedro88 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

please test

@kpedro88

kpedro88 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

The GitHub test box says:

cms/51811/el8_amd64_gcc13/relvals/amd_mi300x — Errors found while running runTheMatrix

but this is not actually reported in the bot's post.

I remain unclear why the GPU tests were automatically triggered for this PR at all.

@kpedro88

kpedro88 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Comparison changes arise in FastSim workflows in Muon-related objects, as expected.

Other changes are an instance of #47071. Tangential though interesting, these changes in TrackerPhase2OTL1Track appear in both 37634.911 (Phase 2 DD4hep workflow) as usual and 37834.0 (Phase 2 PU non-DD4hep workflow), which is the first time I have seen these spurious changes outside of DD4hep (though there are fewer of them). Maybe we have finally perturbed stack memory enough for this to surface in other workflows?

@kpedro88

kpedro88 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

ignore tests-rejected with manual-override

@kpedro88

kpedro88 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

+1

@gabrielmscampos

Copy link
Copy Markdown
Member

+dqm

@kpedro88

kpedro88 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

@cms-sw/pdmv-l2 please check and sign

@antoniovagnerini

Copy link
Copy Markdown

+pdmv

@cmsbuild

Copy link
Copy Markdown
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, @ftenchini (and backports should be raised in the release meeting by the corresponding L2)

@mandrenguyen

Copy link
Copy Markdown
Contributor

+1

@cmsbuild
cmsbuild merged commit ab3b9af into cms-sw:master Sep 12, 2026
54 of 55 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.

7 participants