-
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
Update of GEM & CSC trigger primitive emulators #38373
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38373/30565
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
test parameters: |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38373/30566 |
A new Pull Request was created by @giovanni-mocellin for master. It involves the following packages:
@rekovic, @jpata, @cecilecaillol, @civanch, @yuanchao, @ahmad3213, @cmsbuild, @pmandrik, @epalencia, @emanueleusai, @mdhildreth, @AdrianoDee, @jfernan2, @slava77, @ggovi, @micsucmed, @francescobrivio, @malbouis, @clacaputo, @srimanob, @rvenditti, @tvami can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
Hi @giovanni-mocellin
|
I'm sorry, it's my first PR to central CMSSW, so I'll ask probably trivial questions for probably trivial doubts...
|
No worries, you can still fix it, maybe we can chat on mattermost on what do do exaclty
Maybe @ptcox could chime in here from the CSC side? My proposal is to store these in an sqlite file instead of your 112 txt files in cms-data, and read it through that database, making sure there is a reproducibility for the past (*) and easy updates for the future (*) this actually triggered me a new question: if you delete the files in cms-data, does that mean the Run-2 reconstruction wont be able to be reproduced?
I'd actually prefer to have the full changes tested in the Jenkins here, meaning that if there is no objection to do it in this PR, I'd like you do commit the flag changes here.
That is reassuring, thanks! |
process.GlobalTag.toGet = cms.VPSet( | ||
cms.PSet(record = cms.string("GEMChMapRcd"), | ||
tag = cms.string("GEMChMapRcd"), | ||
connect = cms.string("sqlite_fip:EventFilter/GEMRawToDigi/test/GEMeMap_GE11_b904.db") |
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.
this is under test, so maybe it was just something temporary, but we just updated the GEM ChMap in the GT, how is this tag different? @hyunyong maybe?
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.
This is for testing the trigger features in the b904 laboratory, where we have cosmic stands that require dedicated mapping files. This map file is indeed in line with the new GEM ChMap format
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.
This file is utilized for a cosmic stand in b904 laboratory, where we validate the FW features. The mapping follows the format of the recently introduced GEM ChMap in the GT. Hope this answers the question, otherwise maybe @yeckang could know more.
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.
@tvami
This unpacker is not for the P5. The data should come from the local setup in B904. And as you can expect, the mapping would be different from the one that we are using for the P5.
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.
Ok that's good to know. But then this tag should be uploaded to the DB as a single-iov tag, with an appropriate naming, and then ES prefer that. tagging @hyunyong again for doing the upload
Sure, thanks for your help!
We only modified tables that were not used in Run-2, since both GEM-CSC features and CSC consistency were not used (in fact, CSC consistency will not be used in Run-3 either, but we wanted to keep the feature there)
Will do |
+reconstruction |
+Upgrade Re-sign. Last commit does not change Upgrade part. |
@cms-sw/dqm-l2 @cms-sw/simulation-l2 changes since your latest signature are rather limited: could you please review, and sign if you are fine with it? |
+1 |
@cms-sw/orp-l2 can we bypass the simulation signature? I think Vladimir did sign this already in the past so it should be fine |
see #38373 (comment) |
+1 |
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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
I'm certainly not making a claim like that!
Can "track packing in miniAOD" depend on reco'ed muons? The only CSC
information in even AOD, is AFAIK, only that associated with reco'ed
muons (i.e. CSC rechits and segmnets contributing to reco'ed muons).
Tim
… Marco Musich ***@***.***>
July 4, 2022 at 16:46
I still don't understand how the CSC readout can affect the track
packing in miniAOD, but it's not up to me to sign this PR :)
—
Reply to this email directly, view it on GitHub
<#38373 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGYLHXMO4VILDALVNDOB33VSL2LDANCNFSM5Y2QBMRA>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
PR description:
This PR introduces major changes in the L1 trigger primitive emulators for GEM and GEM-CSC, plus some minor changes for CSC for Run-3 mode. In addition, it ensures the full compatibility of unpackers and emulators to the b904 lab test setups, utilized during the SW and FW validation.
Changes for GEM trigger primitive emulator:
Changes for CSC trigger primitive emulator:
Changes for GEM-CSC trigger primitive emulator:
NOTE 1)
This PR is intrinsically linked to the PR #13 to L1Trigger-CSCTriggerPrimitives, which updates the lookup tables for the GEM-CSC trigger to the ones currently used in FW
NOTE 2)
Update to the GEM-CSC trigger primitive matching algorithm. Before it reflected an old concept of FW implementation, now it adheres completely to the FW implemented and deployed
A few notable changes:
NOTE 3)
In agreement with EMTF experts (@eyigitba), the flags that control the run-3 algorithms in CSC (and EMTF) have been kept off for this PR. Another PR (or a new commit) will come within a few days to abilitate the necessary flags. Since the PR includes changes in the run-3 algorithms, the tests were performed turning on such flags.
=> UPDATE => Flags were set to True with a new commit
PR validation:
The code has been tested and validated by re-emulating b904 lab and P5 data taken during the FW commissioning phase. Plots of comparisons between re-emulation and data show very good agreement. A couple of re-emulation files for a GE1/1-ME1/1 run in b904 and a ME4/2 run in b904 have been attached to this PR.
ReEmuFiles.zip
Note: the tests were done with run-3 flags activated, in order to verify the proper functioning of the implemented algorithms. As described before, now these flags have been turned off, and will be turned on in a second PR (or new commit), to come within a few days.
=> UPDATE => Flags were set to True with a new commit