-
Notifications
You must be signed in to change notification settings - Fork 26
OMTF configuration fix to use phase-2 values #1247
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
OMTF configuration fix to use phase-2 values #1247
Conversation
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. Attempts to compile this PR succeeded!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. I found no issues with the code checks!
I found no issues with the headers!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. I found 2 files that did not meet formatting requirements:
Please run
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. This PR passes available unit tests!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation I found a non-zero return code running the relval workflows for this PR!
|
…es in the OMTF configs
|
Hi @aloeliger @epalencia, this seems to be ready now, from a quick look, the efficiency for GMT SA muons is recovered (running with more stats now) I'd appreciate is @kbunkow and @zhenbinwu could have a look as well. This superseeds #1241 |
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. Attempts to compile this PR succeeded!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. I found no issues with the code checks!
I found no issues with the headers!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. I found 1 files that did not meet formatting requirements:
Please run
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. This PR passes available unit tests!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation I found a non-zero return code running the relval workflows for this PR!
|
| prompt.push_back(samuon); | ||
| else if (mu.hwPtUnconstrained() > 0) | ||
| } | ||
| if (mu.hwPtUnconstrained() > 0) //Assume exculsive, need double check |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove the old comments?
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. Attempts to compile this PR succeeded!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. I found no issues with the code checks!
I found no issues with the headers!
|
|
Hello, I'm triggerDoctor. @aloeliger is testing this script for L1T offline software validation. I found 2 files that did not meet formatting requirements:
Please run
|
|
Unfortunately it is not working for me. It miss the xml file I can't find it from the cms-data : https://github.com/cms-data/L1Trigger-L1TMuon/tree/master/omtf_config |
|
You need to download the cms-data PR too, it’s on the first comment.
El 10 abr 2024, a las 22:27, Zhenbin Wu ***@***.***> escribió:
It doesn't work for me. I got the missing file error:
edm::FileInPath unable to find file L1Trigger/L1TMuon/data/omtf_config/Patterns_ExtraplMB1nadMB2DTQualAndEtaFixedP_ValueP1Scale_t20_v1_SingleMu_iPt_and_OneOverPt_classProb17_recalib2_minDP0.xml anywhere in the search path.
—
Reply to this email directly, view it on GitHub<#1247 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABFM4UZFWVV4CJ4GT6WF52DY4WOA3AVCNFSM6AAAAABF6V7HDSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBYGM3TSMBWHA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
The data files required by this PR have been added into the prototype explicitly with #1250 |
|
@folguera Given the time crunch, we're not going to spend too much time debating the nature of this here provided everything still compiles and we can run our commands. I think we can already go ahead and merge this. Please go ahead and open the PR to central CMSSW, and we'll merge this here. |
|
@aloeliger @epalencia this PR requires #1196 to be merged to central before. |
|
@folguera Can this be added to cms-sw#44557 then? |
|
@folguera @zhenbinwu @epalencia @omiguelc I am going to merge this, #1250, and #1249 when I get back to my apartment in an hour or so. Are there any further expected muon fixes? Are we waiting for anything else to provide to the menu team? |
|
@folguera @zhenbinwu @omiguelc I would like to ask if all these PRs allow the Menu team to use the GMT Standalone collection for the Standalone prompt muon efficiencies/rates, and for the Standalone displaced muon efficiencies/rates. Thanks a lot for confirming. @aloeliger : I think we still expect a PR to fix the GMT TkMuon efficiency at low pT |
|
@cbotta YES, standalone muons, both prompt and (hopefully) displaced can be used by the menu team. |
|
For this PR, the prompt OMTF looks OK. For the displaced OMTF, we need the experts to confirm the efficiency I saw. Thanks |
|
Ok, please merge this PR for the SAMuon. Thanks |
5fb1ece
into
cms-l1t-offline:phase2-l1t-integration-14_0_0_pre3
Add data files required by PR #1247
|
Hi, I have cherry-picked the recent PRs (#1228, #1239, #1249, #1251) to the GMT PR (cms-sw#44498), expect this one. Part of this PR is in the Phase-2 OMTF PR (cms-sw#44557). So I will leave these commits to that PR. |
|
@folguera For our records, I need to understand if this has ended up in a PR to CMSSW, and where it ended up finally. Is there a PR to CMSSW with these commits in them? |
|
Not yet, we need to add them to cms-sw#44557 |
|
Since the OMTF PR need to be merged before the GMT PR, I think it would be more practical to split the changes of I can just commit the changes to the GMT branch, dropping Santi's commit history, if that is OK. |
|
Sounds good
El 22 abr 2024, a las 17:09, Zhenbin Wu ***@***.***> escribió:
Since the OMTF PR need to be merged before the GMT PR, I think it would be more practical to split the changes of L1Trigger/L1TMuonOverlapPhase2/python/fakeOmtfParamsPhase2_cff.py and L1Trigger/L1TMuonOverlapPhase2/python/simOmtfPhase2Digis_extrapol_cfi.py to the OMTF PR, L1Trigger/Configuration/python/SimL1Emulator_cff.py and L1Trigger/Phase2L1GMT/plugins/Phase2L1TGMTFwdMuonTranslator.cc to the GMT PR.
I can just commit the changes to the GMT branch, dropping Santi's commit history, if that is OK.
—
Reply to this email directly, view it on GitHub<#1247 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABFM4U6AFDQ3BBCYMZ7VJ7LY6URZVAVCNFSM6AAAAABF6V7HDSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRZHAYTMNJSGI>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
@zhenbinwu, was this fixed finally ported to CMSSW? If so, where? |
Yes, part of this PR was added to GMT PR cms-sw@642efa5 I think the OMTF part are merged in CMSSW as well. |
PR description:
This contains the required config files to load the correct OMTF configuration for Phase-2. This is built on top of #1241 and it requires cms-data/L1Trigger-L1TMuon#27 to be merged and downloaded inside L1Trigger/L1TMuon/data/omtf_data
PR validation:
First quick look at efficiencies look OK: https://folguera.web.cern.ch/L1T/2024_04_AR_Tests/V36nano_muons_test/object_performance/turnons/MuonTFsMatching_Eta_Pt15toInf_-999_V36nano_muons.png