-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 GEM unpacker in L1Repack FullMC and temporary load of GEM geometry to hltGetConfiguration #34788
Add GEM unpacker in L1Repack FullMC and temporary load of GEM geometry to hltGetConfiguration #34788
Conversation
Still same conflict - need to start from most recent 12_1 IB.... |
Should be OK now. I started from CMSSW_12_1_X_2021-08-04-1100, maybe the last merge was after that. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34788/24487
|
A new Pull Request was created by @srimanob (Phat Srimanobhas) for master. It involves the following packages:
@perrotta, @cmsbuild, @silviodonato, @Martin-Grunewald, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Please test |
d7c1197
to
c6c934e
Compare
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34788/24518
|
Pull request #34788 was updated. @perrotta, @cmsbuild, @silviodonato, @Martin-Grunewald, @qliphy, @fabiocos, @davidlange6 can you please check and sign again. |
@cmsbuild Please test |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34788/25313
|
Pull request #34788 was updated. @perrotta, @cmsbuild, @Martin-Grunewald, @qliphy, @fabiocos, @davidlange6 can you please check and sign again. |
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1eadd7/18649/summary.html Comparison SummarySummary:
|
Is there a dd4hep test where it fails in case the customisation is run? |
Hi Martin, yes we have. |
Yes, but that was before we used: |
Ah, my local test also shows this when I switched to |
OK, I reproduced the problem. However, as you know, we are later going to put this (ES module) directly into the menu in ConfDb and the associated menu dumps, but this does not know about dd4hep vs non-dd4hep. So I am afraid I can not sign off on this PR; the problem must be solved at dd4hep/non-dd4hep level. I note no other ES module needs this special case. |
-1 |
No problem. This PR is on temporary to make GEM unpacking. If it is not OK on HLT side, we can wait. The original reason of this PR is to make sure we can do fullMC emulation. I hope when we have it integrated with CSC, we should not see any issues. I still have no idea why it is only GEM module which is called twice. |
PR description:
It was #34785
This PR is to fix two issues when L1REPACK is used for Run3. The error messages w/o this PR is mentioned at the end of the PR description.
(1) no GEM unpacker defined in SimL1EmulatorRepack. It is solved in this PR.
(2) when hltGetConfiguration is used, GEM geometry is not loaded properly. This PR introduced a temporary fix.
Next step:
Following the comment on #34785 (comment)
PR validation:
To test (1), the following cmsDriver run fine.
cmsDriver.py --process reHLT -s L1REPACK:FullMC,HLT:@relval2021 --conditions auto:phase1_2021_realistic --datatier GEN-SIM-RAW -n -1 --eventcontent RAWSIM --geometry DB:Extended --era Run3 --filein file:step2.root --fileout file:step3_reHLT.root --python reHLT_ProdLike_2021.py --no_exec
To test (2), the following config can run fine.
hltGetConfiguration /dev/CMSSW_12_1_0/GRun/V1 --process reHLT --globaltag auto:phase1_2021_realistic --mc --unprescale --paths HLTriggerFirstPath,HLT_IsoMu24_v13,HLT_Mu50_v13,HLTriggerFinalPath --output minimal --input file:step2.root --l1-emulator FullMC > tmp.py
Test cover also DD4hep (wf 11634.911), as
GEMGeometryESModule
will be loaded twice.if this PR is a backport please specify the original PR and why you need to backport that PR:
Not a backport, and no need of backport
Note
Issue of (1): solved in this PR
Issue of (2): temporary solution is provided for now