Skip to content

[GEM] Turning on the applyMasking flag for Run 3 GEM data taking #47257

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

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

yeckang
Copy link
Contributor

@yeckang yeckang commented Feb 4, 2025

PR description:

  • GEM strip masking was started in the middle of 2023 to avoid a high trigger rate which is caused by noisy channels
  • This PR enables clustering by considering the strip masking on the local reconstruction of GEM.
  • The PR couldn't be made because no GEMDeadSrtripsRcd and GEMMaskedStripsRcd existed in any global tag.
  • The GEMDeadStripsRcd and GEMMaskedStripsRcd have been on the condDB recently.
  • At the moment, the GEMDeadStripsRcd and GEMMaskedStripsRcd are empty. So we don't expect any changes in the result.

PR validation:

  • The scram b code-format and scram b code-checks are applied.
  • The runTheMatrix.py -w standard -l 145.006 is performed for the test. The scenario using the Run3 data which is this PR affecting.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2025

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2025

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

It involves the following packages:

  • RecoLocalMuon/GEMRecHit (upgrade, reconstruction)

@Moanwar, @cmsbuild, @jfernan2, @mandrenguyen, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks.
@bellan, @jhgoh, @jshlee, @missirol, @watson-ij 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 Feb 4, 2025

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 4, 2025

-1

Failed Tests: UnitTests RelVals AddOn
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-01f41a/44188/summary.html
COMMIT: 6180524
CMSSW: CMSSW_15_0_X_2025-02-04-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47257/44188/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 2 errors in the following unit tests:

---> test test_MC_22_crosscheck had ERRORS
---> test test_MC_23_crosscheck had ERRORS

RelVals

----- Begin Fatal Exception 04-Feb-2025 19:16:23 CET-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module GEMRecHitProducer/'gemRecHits'
Exception Message:
No "GEMMaskedStripsRcd" record found in the EventSetup.

 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 04-Feb-2025 19:20:56 CET-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module GEMRecHitProducer/'gemRecHits'
Exception Message:
No "GEMMaskedStripsRcd" record found in the EventSetup.

 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 04-Feb-2025 19:24:25 CET-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  stream begin Run run: 1 stream: 0
   [1] Calling method for module GEMRecHitProducer/'gemRecHits'
Exception Message:
No "GEMMaskedStripsRcd" record found in the EventSetup.

 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------
Expand to see more relval errors ...

AddOn Tests

----- Begin Fatal Exception 04-Feb-2025 18:59:07 CET-----------------------
An exception of category 'NoRecord' occurred while
   [0] Processing  stream begin Run run: 1 stream: 1
   [1] Calling method for module GEMRecHitProducer/'gemRecHits'
Exception Message:
No "GEMMaskedStripsRcd" record found in the EventSetup.

 Please add an ESSource or ESProducer that delivers such a record.
----- End Fatal Exception -------------------------------------------------

@mmusich
Copy link
Contributor

mmusich commented Feb 4, 2025

FYI @cms-sw/alca-l2 looks like some GTs to be updated were missed (e.g. HIon MC, phase-2)

@watson-ij
Copy link
Contributor

FYI @cms-sw/alca-l2 looks like some GTs to be updated were missed (e.g. HIon MC, phase-2)

@yeckang probably we don't need this in phase 2, can we just turn off masking in the phase2 modify line?

@perrotta
Copy link
Contributor

perrotta commented Feb 5, 2025

FYI @cms-sw/alca-l2 looks like some GTs to be updated were missed (e.g. HIon MC, phase-2)

@yeckang probably we don't need this in phase 2, can we just turn off masking in the phase2 modify line?

I will prepare the modified GTs with the dedicated tags nonetheless

@yeckang
Copy link
Contributor Author

yeckang commented Feb 5, 2025

I thought that the applyMasking flag should be false if I don't make a modifier. But it does not work the way I thought. I will push the new commit with the modifier for phase 2. I don't plan to turn on the flag for phase 2.
And I will contact AlCa to include the tags for HIon.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 5, 2025

@perrotta
Copy link
Contributor

Great, tests succeed!
2025 GTs already contain all needed features.
There are no differences in the test outputs because currently there are not masked channels in GEM in those payloads.

@jfernan2
Copy link
Contributor

+1
@perrotta I understand this is ready to go then, I have not assigned to alca-db this PR, but feel free to autoassign if you consider it. Thanks

@Moanwar
Copy link
Contributor

Moanwar commented Feb 20, 2025

+Upgrade

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

@perrotta
Copy link
Contributor

assign alca

@cmsbuild
Copy link
Contributor

New categories assigned: alca

@atpathak,@consuegs,@perrotta you have been requested to review this Pull request/Issue and eventually sign? Thanks

@perrotta
Copy link
Contributor

+alca

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

@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 4963dad into cms-sw:master Feb 20, 2025
11 checks passed
@missirol
Copy link
Contributor

@yeckang

Does this update need to be backported to CMSSW_15_0_X ?

@missirol
Copy link
Contributor

@yeckang

Does this update need to be backported to CMSSW_15_0_X ?

Tagging @cms-sw/reconstruction-l2 and @cms-sw/alca-l2 before I forget this.

This change has been applied to the Run-3 HLT menus for 2025 in CMSHLT-3420.

@perrotta
Copy link
Contributor

@yeckang
Does this update need to be backported to CMSSW_15_0_X ?

Tagging @cms-sw/reconstruction-l2 and @cms-sw/alca-l2 before I forget this.

This change has been applied to the Run-3 HLT menus for 2025 in CMSHLT-3420.

... and the 150X online GTs are ready for it! This PR does not affect the possibility to implement it in the HLT menu. Of course, if GEM wants to have the possibility to mask or declare as dead some channels also in reco, @yeckang (or someone else from the GEM group) should take care of backporting this PR in 150X.

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.

9 participants