Updates L1T OMTF for Phase 2#50032
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50032/47864
|
|
A new Pull Request was created by @mbluj for master. It involves the following packages:
@BenjaminRS, @cmsbuild, @quinnanm can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
-1 Failed Tests: ClangBuild Failed Clang BuildI found compilation warning while trying to compile with clang. Command used: See details on the summary page. |
|
Fixed issue found in tests with clang. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50032/47899
|
|
Pull request #50032 was updated. @BenjaminRS, @cmsbuild, @quinnanm can you please check and sign again. |
|
Please test this PR with cms-data/L1Trigger-L1TMuon#29. |
|
I'd like to ask to start tests and eventually the review, thank you. |
…of cast from float to int
The bug has had no impact as it was part of the final, redundant condition in an "else if" statement
e.g. better use of const, cout=>MessageLogs, additional comments, etc.
OMTFProcessor.cc and OmtfProcessorPhase2.cc Fixing the condition to assign minimum pT (0 or 1 GeV) for the muons that have only unconstrained pt valid. Checking getFiredLayerCntConstr was missing. updating runMuonOverlap_run3_mc.py and runMuonOverlap.py
RegionalMuonCandBxCollection is no longer produced by L1TMuonOverlapPhase2TrackProducer, RegionalMuonCand are not created by the OmtfEmulation. Only SAMuonCollection (OMTFconstr and OMTFunconstr) are produced now. L1Trigger/Phase2L1GMT/plugins/Phase2L1TGMTFwdMuonTranslator.cc was modified to avoid using RegionalMuonCand and instead use SAMuon from OMTF. L1Trigger/L1TMuonOverlapPhase1/src/Omtf/OMTFConfiguration.cc: fix in procPhiOmtfToGlobalPhiOmtf
|
The PR was updated implementing review comments and rebased to a newer IB of 21 June. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50032/49898 |
|
Pull request #50032 was updated. @BenjaminRS, @cmsbuild, @quinnanm can you please check and sign again. |
|
please test |
|
+1 Size: This PR adds an extra 16KB to repository The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see more details here: Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 1 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
BenjaminRS
left a comment
There was a problem hiding this comment.
I think we can approve this PR as is. But we would like a further PR to tidy up the code a bit more
| #'/store/express/Commissioning2021/ExpressCosmics/FEVT/Express-v1/000/344/266/00000/db2cfbdd-5edf-4ee4-aab0-5bdba105728d.root' #DT and RPC fw 0x0008 | ||
| #'/store/express/Commissioning2021/ExpressCosmics/FEVT/Express-v1/000/344/566/00000/19ef107a-4cd9-4df0-ba93-dbfbab8df1cb.root' | ||
| #'/store/express/Commissioning2021/ExpressCosmics/FEVT/Express-v1/000/347/053/00000/7b486245-96ea-4b7c-9fe7-76c957968785.root' #RPC noise only | ||
| 'file:/eos/home-k/kbunkow/cms_data/run3_data/Run2025G_398240_Muon0_raw.root' |
There was a problem hiding this comment.
I think we can leave it for now, but better to have these sort of files in some central EOS/L1T area
| #process.load('Configuration.StandardSequences.RawToDigi_Data_cff') | ||
| #process.load('EventFilter.L1TRawToDigi.omtfStage2Digis_cfi') #unpacker | ||
|
|
||
| #process.load('Configuration.StandardSequences.FrontierConditions_GlobalTag_cff') |
There was a problem hiding this comment.
In a future PR can we clear up the commented out code please?
| #process.load('Configuration.EventContent.EventContent_cff') | ||
| #process.load('SimGeneral.MixingModule.mixNoPU_cfi') | ||
| process.load('Configuration.Geometry.GeometryExtendedRun4D95Reco_cff') | ||
| process.load('Configuration.Geometry.GeometryExtendedRun4D110Reco_cff') |
There was a problem hiding this comment.
Geometries should be D121 now
| #process.load('Configuration.EventContent.EventContent_cff') | ||
| #process.load('SimGeneral.MixingModule.mixNoPU_cfi') | ||
| process.load('Configuration.Geometry.GeometryExtendedRun4D95Reco_cff') | ||
| process.load('Configuration.Geometry.GeometryExtendedRun4D110Reco_cff') |
| {"path": "/eos/user/a/akalinow/Data/SingleMu/12_5_2_p1_04_04_2023/SingleMu_ch0_iPt2_12_5_2_p1_04_04_2023/", "fileCnt" : 500}, #500 files | ||
| {"path": "/eos/user/a/akalinow/Data/SingleMu/12_5_2_p1_04_04_2023/SingleMu_ch2_iPt2_12_5_2_p1_04_04_2023/", "fileCnt" : 500}, #500 files |
There was a problem hiding this comment.
Better to use a L1T/Muon EOS space
| #process.load('FWCore.MessageService.MessageLogger_cfi') | ||
| #process.load('Configuration.EventContent.EventContent_cff') | ||
| #process.load('SimGeneral.MixingModule.mixNoPU_cfi') | ||
| process.load('Configuration.Geometry.GeometryExtendedRun4D110Reco_cff') |
|
+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. @mandrenguyen, @ftenchini, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2) |
|
Thank you @BenjaminRS. |
|
@mbluj - I think you can do this in a follow up PR. For the Geometries, did you choose those due to a specific file you were using that was e.g. created with D110? |
I would push these "cosmetic" changes here, but fine if you prefer to include them to a bigger cleaning PR. Anyway, I will fix the typo in the 170X backport PR. As for geometry: the one used was recommended when the files were created in a private development area, but OMTF output is almost insensitive on this. Updating to the current default makes sense. |
|
OK - lets just quickly change here then and we can re-test and sign today |
…n to match those defined in the upstream producer
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-50032/50027 |
|
Pull request #50032 was updated. @BenjaminRS, @cmsbuild, @quinnanm can you please check and sign again. |
|
please test |
|
+1 Size: This PR adds an extra 32KB to repository Comparison SummarySummary:
Max Memory Comparisons exceeding threshold@cms-sw/core-l2 , I found 1 workflow step(s) with memory usage exceeding the error threshold: Expand to see workflows ...
|
|
+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. @sextonkennedy, @ftenchini, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR introduces recent (as for beginning of 2026) changes in the L1T OMTF emulator for Phase2:
SAMuons(Phase 2 data format):FinalMuonwas introduced that stores the candidate parameters in the output format. It can be either Phase 1 or Phase 2 scales. Then it is converted toRegionalMuonCands(Phase 1) orSAMuons(Phase 2);CandidateSimMuonMatcherandDataROOTDumper2used for developments and validations.In addition the PR contains a fix of bugs found in
DTTriggerPhase2which are already included in #49589, but as it is not merged yet added here for self-consistency.Details in this JIRA ticket and K. Bunkowski's presentation here.
This PR requires updated configuration files provided in cms-data/L1Trigger-L1TMuon#29.
PR validation:
Validated with Phase 2 workflows, e.g. 23634.0.
FYI, @akalinow, @kbunkow