Adding HCAL FB LUT to caloParams for uploading to DB #41221
Conversation
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41221/34939 |
|
A new Pull Request was created by @vshang for master. It involves the following packages:
@epalencia, @cmsbuild, @cecilecaillol, @aloeliger can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41221/34944 |
|
Pull request #41221 was updated. @epalencia, @cmsbuild, @cecilecaillol, @aloeliger can you please check and sign again. |
|
@vshang Am I misremembering? When we did this last the ALCA experts told us that |
|
@vshang, Ahh, okay never mind, too tired and not paying enough attention, I see where the change is made and what you've done. Do we need to tag in the |
Sure, wouldn't hurt to have them check again to confirm nothing breaks from their side. |
|
This should be ok, the |
Yes, I got around the issue by just saving the upper and lower halves of the 64 HCAL FB LUT as separate entries in the caloParams file, then combining the two later on when writing the LUT to a config file. The output config file has not changed, just how it is stored in the caloParams file, which means no additional changes need to be made to the DB or SWATCH. |
|
please test |
|
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3f0c10/31886/summary.html Comparison SummarySummary:
|
|
Looks like the logs and trigger results are fine. |
Where is this activated then? I'm glad that everything passes now, but am I correct in reading this as you essentially just getting rid of the changes (if not the code) that you just worked so hard to introduce? |
No, everything still works as expected for generating the CaloL1 algo settings key for uploading to DB from the updated caloParams_2023_v0_0_cfi file, which I've tested locally. There is a private repository maintained at [1] which is used to generate this key and requires useHCALFBLUT = true (see [2]). Nothing else in the rest of CMSSW should be affected; the rest of the commits were to ensure this was the case. [1] https://github.com/uwcms/L1TCaloLayer1LUTWriter |
|
Then for my edification, does this need to be enabled in CMSSW anywhere? Is it going to cause emulation mismatches or something with it disabled? (But presumably present in hardware?) |
The HCAL FB LUT parameter is already included in the current CaloL1 algo settings key used for production and has already been present in the CaloL1 CTP7's since the LLP trigger was first introduced. Nothing should be expected to change with CMSSW or emulation comparison to hardware. Previously the HCAL FB LUT was loaded manually through SWATCH commands directly to the firmware. This PR is just to ensure that this can be done by changing the values in the caloParams file and uploading the changes to DB instead. To answer your question more directly, I don't expect the HCAL FB LUT parameter to be enabled in CMSSW elsewhere, and this PR should not change anything with regard to emulation comparisons to hardware. |
|
Okay. It's for custom plugin usage, I guess I get it. Do we understand what things would have to change to prevent the compatibility issues we saw? |
|
Do you mean changes in addition to this PR? None that I am aware of. |
I meant to prevent the failing tests and trigger mismatches. |
Ah yes, those should not be an issue anymore. I squashed the changes together so it might be hard to pick out, but this is handled essentially by disabling the check for the new parameters unless explicitly enabled (useHCALBLUT = true). |
Right, but my point is, should we have a list of the things that are not compatible with the new HCAL LUT and fix those? |
I don't think so, though I can confirm with my supervisors. Given that the LLP trigger logic is not expected to change unless there is a major update from the HCAL side, I think keeping everything else the same without changes is probably the best way forward for now. |
|
+l1
|
|
+db |
cmsbuild
added
fully-signed
db-approved
and removed
db-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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR is to add a parameter to the latest caloParams file (caloParams_2022_v0_6_cfi.py) containing the HCal FB LUT for the LLP trigger as well as modifications to CaloL1 software to handle converting the HCal FB LUT from the caloParams file to the CaloL1 Algo settings key to upload to the DB.
PR validation:
Changes were tested locally on CMSSW_13_1_0_pre2 release by using the modified caloParams file to produce a CaloL1 Algo settings key containing the correct HCal FB LUT. This Algo settings key has been tested and validated during several P5 tests together with the new CaloLayer1 firmware and SWATCH software to upload the HCal FB LUT from DB to firmware.