Calo-L1 unpacker modified to read 5BX payload format #36259
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36259/26919
|
|
A new Pull Request was created by @hftsoi (Ho-Fung Tsoi) for master. It involves the following packages:
@cmsbuild, @rekovic, @cecilecaillol can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a5271c/20821/summary.html Comparison SummarySummary:
|
| bool negativeEta = false; | ||
| if (cEta < 0) | ||
| negativeEta = true; |
There was a problem hiding this comment.
| bool negativeEta = false; | |
| if (cEta < 0) | |
| negativeEta = true; | |
| bool negativeEta = (cEta < 0); |
| bool negativeEta = false; | ||
| if (cEta < 0) | ||
| negativeEta = true; |
There was a problem hiding this comment.
| bool negativeEta = false; | |
| if (cEta < 0) | |
| negativeEta = true; | |
| bool negativeEta = (cEta < 0); |
| bool negativeEta = false; | ||
| if (side == 0) | ||
| negativeEta = true; |
There was a problem hiding this comment.
| bool negativeEta = false; | |
| if (side == 0) | |
| negativeEta = true; | |
| bool negativeEta = (cEta < 0); |
| bool negativeEta = false; | ||
| if (side == 0) | ||
| negativeEta = true; |
There was a problem hiding this comment.
| bool negativeEta = false; | |
| if (side == 0) | |
| negativeEta = true; | |
| bool negativeEta = (cEta < 0); |
| uint32_t lEta = 10 - region; // GCT eta goes 0-21, 0-3 -HF, 4-10 -B/E, 11-17 +B/E, 18-21 +HF | ||
| if (!negativeEta) | ||
| lEta = region + 11; |
There was a problem hiding this comment.
| uint32_t lEta = 10 - region; // GCT eta goes 0-21, 0-3 -HF, 4-10 -B/E, 11-17 +B/E, 18-21 +HF | |
| if (!negativeEta) | |
| lEta = region + 11; | |
| // GCT eta goes 0-21, 0-3 -HF, 4-10 -B/E, 11-17 +B/E, 18-21 +HF | |
| uint32_t lEta = negativeEta? 10 - region: region + 11; |
| uint32_t tower = iPhi; | ||
| if ((cEta % 2) == 0) | ||
| tower += 4; | ||
| if (cType == HF) { | ||
| tower = (cEta - 30); | ||
| if (cEta == 41) | ||
| tower = 10; | ||
| } |
There was a problem hiding this comment.
| uint32_t tower = iPhi; | |
| if ((cEta % 2) == 0) | |
| tower += 4; | |
| if (cType == HF) { | |
| tower = (cEta - 30); | |
| if (cEta == 41) | |
| tower = 10; | |
| } | |
| uint32_t tower; | |
| if (cType == HF) { | |
| tower = (cEta != 41? cEta - 30: 10); | |
| } else { | |
| tower = iPhi; | |
| if ((cEta % 2) == 0) | |
| tower += 4; | |
| } |
Thanks for your review and suggestion! I guess at this stage we hope to keep the duplicated parts (for 5BX unpacking only) with changes as minimal as possible compared to the original (for normal unpacking only), even if it is for code optimization, just to have a more consistent comparison in case things arise. We will consider it later for both duplicated and original parts the same time. |
|
+l1 |
cmsbuild
added
fully-signed
l1-approved
and removed
l1-pending
pending-signatures
labels
|
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 (and backports should be raised in the release meeting by the corresponding L2) |
|
@hftsoi Ok, please keep in mind the comments when you change both the original and duplicated parts |
|
+1
|
[backport of #36259] Calo-L1 unpacker modified to read 5BX payload format
PR description:
This PR is about the trigger Calo-L1 unpacker modified to read ECAL's 5BX FAT events properly. The current central unpacker for FAT events is assuming a payload size of 192 * 32-bit words and the same structure as for normal events, repeated itself one after another five times for each FAT event, and each Calo fragment contains one BX only. However this is not compatible with the actual current payload format which has a larger size of 5 * 192 * 32-bit words, where each Calo fragment contains all 5 BX already, stacked in the order BX-2, -1, 0, +1, +2 inside.
The modification involves duplication of all necessary unpacker functions from normal events unpacking, then to make changes there for FAT events unpacking. The idea is to have two separate unpackers, one for unpacking normal events only, and another for unpacking FAT events only, in order to minimize risk.
Note: UCTCTP7RawData5BX.h is duplicated from UCTCTP7RawData.h, which contains the index equations for locating Calo fragments in payload data, offsets are modified for 5BX, while geometry, word structure and etc. remain the same.
PR validation:
Validated by running private online DQM on recent CRUZET runs, all DQM plots looked expected with modified unpacker. Also asked the central DQM team to include these changes into temporary production for this last week of running, all tests ran fine and DQM plots looked expected.