-
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
Modify RPC and GEM unpacker blocks in EMTF unpacker to match the new Run 3 format #39388
Modify RPC and GEM unpacker blocks in EMTF unpacker to match the new Run 3 format #39388
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39388/32092
|
A new Pull Request was created by @eyigitba 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 |
Please test |
Since this is needed for data taking after TS1 and LHC recovery, please open a backport to Marco(ORM) |
urgent |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-36c7f7/27531/summary.html Comparison SummarySummary:
|
+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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
@@ -55,6 +55,15 @@ namespace l1t { | |||
uint16_t RPCc = payload[2]; | |||
uint16_t RPCd = payload[3]; | |||
|
|||
// Run 3 has a different EMTF DAQ output format since August 26th |
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.
What is the purpose of this commented out block? Please either add a comment which explains its usage, or remove it
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 was my bad. I forgot to remove that part :). Should be fixed now
Pull request #39388 was updated. @epalencia, @cmsbuild, @cecilecaillol, @rekovic can you please check and sign again. |
please test |
+l1 |
This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-36c7f7/27550/summary.html Comparison SummarySummary:
|
+1 |
@perrotta I can see the protections are still there at https://github.com/cms-sw/cmssw/pull/39388/files#diff-1a6dadb65e6c79d8a2fc124b34a859b8e8536d39e2cdfea6d7d9f05eba4a3bddR262-R269 Are you talking about these? |
Ah, ok, thank you for having checked. The confusion was caused by the fact that PR #38961 was never backported in 12_4_X, as its modifications are only there in 12_5_X and 12_6_X That PR adds the protections in the following files:
The backport of this PR adds those protections in EMTFBlockGEM.cc and EMTFBlockRPC.cc, but as such there is no protection for EMTFBlockME.cc and EMTFBlockSP.cc I think that they could also be needed in 12_4_X, and therefore a partial backport of #38961 with what's missing should be prepared for 12_4_X: @eyigitba what do you think? |
PR description:
This PR implements the necessary changes to unpack the new DAQ format that is implemented at P5 a couple of weeks ago. Our tests show that the new DAQ format significantly reduces EMTF dead time at very high PU scenarios. According to the new format 2 RPC (GEM) TPs are packed into 1 RPC (GEM) data block, thus reducing the total number of blocks in EMTF DAQ stream.
Modified
EMTFBlockGEM.cc
,EMTFBlockRPC.cc
to add functionality to create 2 TPs per block depending on the firmware version. Changes inEMTFSetup.cc
are made to pass firmware version to the RPC and GEM block unpackers.A backport of this PR to 12_4_X and 12_5_X is required for upcoming collision runs.
PR validation:
Tested with unpacking data collected last week and running
l1tstage2_dqm_sourceclient-live_cfg.py
to confirm the DQM plots are as expected.If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for: