Remove Rundant Condition Check in HCAL TP Algorithm Setup Code
PR description:
This PR removes a small conditional check in the HCAL trigger primitive algorithm setup code. The removed conditional check is redundant with nearby logic, and causes issues in Phase2 workflows when lastHERing is 0, whereby the legacy, $\leq$ Run2 algorithm is always selected, regardless of HcalTPChannelParameters content. No changes are expected for < Phase2 workflows, and non-trivial changes are expected for Phase2 workflows.
PR validation:
Compiles and a private Phase2 workflow runs without issue.
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:
cms-bot internal usage
+code-checks
Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-48311/45180
A new Pull Request was created by @JHiltbrand for master.
It involves the following packages:
- SimCalorimetry/HcalTrigPrimProducers (l1)
@BenjaminRS, @cmsbuild, @quinnanm can you please review it and eventually sign? Thanks. @abdoulline, @bsunanda, @missirol, @mmusich, @rovere, @sameasy this is something you requested to watch as well. @antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.
cms-bot commands are listed here
Private single-pion comparison (eta-phi CMS scan with 50 GeV pions) of
- this fix of Phase2 TPs
- previous (no PU subtraction) 15_1_0_pre2
test parameters:
- workflow = 29634.78
- relvals_opt = --what upgrade
please test
+1
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e27ec6/46767/summary.html
COMMIT: c58fb91ddecda4d297a35e0ca957649d2d08c668
CMSSW: CMSSW_15_1_X_2025-06-16-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/48311/46767/install.sh to create a dev area with all the needed externals and cmssw changes.
Comparison Summary
Summary:
- You potentially removed 1 lines from the logs
- Reco comparison results: 38 differences found in the comparisons
- DQMHistoTests: Total files compared: 51
- DQMHistoTests: Total histograms compared: 4181335
- DQMHistoTests: Total failures: 188
- DQMHistoTests: Total nulls: 0
- DQMHistoTests: Total successes: 4181127
- DQMHistoTests: Total skipped: 20
- DQMHistoTests: Total Missing objects: 0
- DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
- Checked 220 log files, 188 edm output root files, 51 DQM output files
- TriggerResults: found differences in 6 / 49 workflows
Hi @JHiltbrand -- out of curiosity why with this PR would we see less TPs in the HCAL Barrel? (If I am reading that plot correctly)
Hi @JHiltbrand -- out of curiosity why with this PR would we see less TPs in the HCAL Barrel? (If I am reading that plot correctly)
Hi @BenjaminRS ,
If I understand the selection for that TP plot correctly it is all TPs with $E_T>0.0$ GeV. So when going from the legacy $\leq$ Run2 algorithm to the $\geq$ Run3 algorithm (which is being configured properly from this PR) I am guessing we lose 0.5 GeV TPs to zero. As for why, possibly due to the OOT PU subtraction mechanism (would have to see 8TS data frames for these noPU $\pi$-gun sample), and also that the Run3 algorithm is a 1TS scheme whereas the legacy algorithm is a 2TS scheme, thus there is a difference in the pulse containment correction factor.
@BenjaminRS
can it be approved and go to the release?
Hi @abdoulline - in fact our L1T menu group is just running some tests with and without this code modification to see the differences. The previous tests were not properly running the code. We hope to have an answer soon.
How is it going with this? It would be good to close 15_1_X cycle with this in (or the right fix).
Hi, we've now investigated the impact of this PR on the level-1 phase-2 menu and object performance, and have uploaded summary slides here: https://cernbox.cern.ch/s/t3rl5BYJEkIwj63
In the default L1T CMSSW recipe we don't rederive the HCAL TPs, so there are three versions to compare here:
- "noHCALStep" are the results we get when running the default recipe, where the HCAL TPs are taken from the original Spring24 MC production in CMSSW 140X (dotted lines)
- "rerunHCALStep" are the results we get when we rederive the HCAL TPs in 151X before this PR is implemented (dashed lines): the customisation we use for this is on Slide 5
- "newHCALStep" are the results we get when we rederive the HCAL TPs in 151X after this PR is implemented (solid lines)
Essentially, changing the HCAL TPs only impacts the barrel, and only objects that use the HCAL (as expected): taus jets and sums. There are some small changes for electrons and muons, but these can be ignored as they are caused by unrelated CRAB issues when producing the input NanoAODs used in the studies.
The main impact is in the energy scale of the trigger turn-on plots, which shifts to lower energies than the "noHCALStep" baseline for the "rerunHCALStep" results, but to higher energies for the "newHCALStep" when this PR is implemented. I have explicitly uploaded the HT trigger turn-on plot (350 GeV cut applied to the online HT sum pT, derived from the TT 200 PU sample) from slide 29 as an example of this effect. Are these changes expected?
Hi @RobertJWard ,
Thanks for the validation results and explanation; I think this makes sense.
For the dotted line, that would be where the "legacy" $<\text{Run3}$ TP algorithm was being configured in Phase-2 workflows. Moving to the dashed line, although this does not pick up the PR in question here, it implicitly picks up my previously-merged (and turned out to be) incomplete PR for configuring the Run3 TP algorithm for Phase-2 workflows. Due to the incomplete (incorrect) configuration, the result is an excess of TPs and TP energy is biased high, and thus the $H_T$ turn-on occurs at a lower Gen. $H_T$. Finally, going to the solid line, by including the PR here, the TP algorithm is correctly configuring the Run3 algorithm for Phase2 workflows, and the turn-on is similar to that for the dotted line. The solid line is shifted to a little higher Gen $H_T$ as TP $E_T$ is on average a little lower with the Run3 algorithm due to OOT PU subtraction and the algorithm being a 1-time-sample algorithm as opposed to 2-time-sample.
+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. @sextonkennedy, @ftenchini, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2)
+1