-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Fix incorrect hadronic shower packing and enable global shower unpacking for both uGMT and uGT #36877
Fix incorrect hadronic shower packing and enable global shower unpacking for both uGMT and uGT #36877
Conversation
We were generating this error by looking at the wrong bits.
Two out of six words from the payload are used for showers, we were unfortunately off by one.
Also make error message if we're in danger of overwriting data with muon shower info more useful.
This didn't actually break anything, but caused confusing errors when trying to pack (always empty) showers for OMTF.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36877/28138
|
A new Pull Request was created by @dinyar (Dinyar Rabady) for master. It involves the following packages:
@epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d76dc5/22218/summary.html Comparison SummarySummary:
|
Looked through the DQM plots and while I'm not surprised that we see the changes we see I'll have a quick look offline. |
Ok, stared at this for a while and I believe this is compatible with what was changed, however it looks to me like EMTF still isn't unpacking displacement information (and also doesn't have them as DQM plots). Can you confirm that, @eyigitba? |
@dinyar we still haven't put in the unpacker changes and displaced pT + dxy DQM plots in CMSSW. I was hoping to put them in this week, but I still need some information from Alex. I expect to put them in as soon as possible. |
Hi Efe, Ok, no problem. Do you think it's worth waiting for your changes with this PR to make sure things are fine? If you think it might take a bit too long we can also go ahead with this and then just make sure we check for mismatches once you make your PR.. Cheers, |
Hi Dinyar, I think it's ok to merge this. It shouldn't take long for me the submit the PR, but I don't think it's a reason to wait :). We can check for mismatches once we get there. |
Great, sounds good thanks for the quick answer! @cecilecaillol or @epalencia, could you check this PR and sign if it's fine with you? |
+l1 |
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) |
+1
|
PR description:
This PR groups a few fixes (please let me know if I should split it into one PR per fix):
Two mostly aesthetic fixes:
MuonShower
data format was made a bit more descriptive:PR validation:
Ran on sample with hadronic showers[1] and confirmed that they appear at the output of the uGMT both in the EDM file and L1TNtuple.
[1]
cmsDriver.py l1Ntuple --python_filename mc.py -s L1REPACK:FullMC,RAW2DIGI --mc -n -1 --conditions auto:phase1_2021_realistic --era Run3 --customise=L1Trigger/L1TNtuples/customiseL1Ntuple.L1NtupleRAWEMU --customise=L1Trigger/Configuration/customiseUtils.L1TGlobalMenuXML --filein /store/mc/Run3Summer21DRPremix/HTo2LongLivedTo4b_MH-1000_MFF-450_CTau-100000mm_TuneCP5_14TeV-pythia8/GEN-SIM-DIGI-RAW/120X_mcRun3_2021_realistic_v6 -v2/2550000/e28c777d-10fd-4100-b230-c42b0160ad8a.root