modify HCAL output LUT for saturated TPs with |ieta|>=21 #37745

Michael-Krohn · 2022-04-29T19:11:34Z

PR description:

This PR updates the output LUT for the HCAL Trigger Primitives (TPs). Specifically, it modifies the LUT for TPs with |ieta|>=21 that have a saturated energy of 64 GeV and changes their energy to 128 GeV so that no TPs underestimate the energy. The studies supporting this change can be seen on slide 10 in the following slides from the L1 trigger workshop: indico.

PR validation:

A basic technical test was performed: runTheMatrix.py -l limited -i all --ibeos

if this PR is a backport please specify the original PR and why you need to backport that PR:

This PR is not a backport

cmsbuild · 2022-04-29T19:18:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37745/29610

This PR adds an extra 12KB to repository

cmsbuild · 2022-04-29T19:18:31Z

A new Pull Request was created by @Michael-Krohn for master.

It involves the following packages:

CalibCalorimetry/CaloTPG (l1, alca)

@malbouis, @yuanchao, @epalencia, @cmsbuild, @rekovic, @tvami, @cecilecaillol, @francescobrivio can you please review it and eventually sign? Thanks.
@simonepigazzini, @mmusich, @thomreis, @tocheng this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

tvami · 2022-04-29T19:42:24Z

CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc

@@ -83,6 +83,9 @@ void CaloTPGTranscoderULUT::loadHCALCompress(HcalLutMetadata const& lutMetadata,
      for (unsigned int i = threshold; i < lutsize; ++i)
        if (allLinear_) {
          outputLUT_[index][i] = isOnlyQIE11(id) ? linearQIE11LUT[i] : linearQIE8LUT[i];
+          if (abs(ieta) > 20 && isOnlyQIE11(id) && linearQIE11LUT[i] >= 127) {


should these numbers be made a name constant? what's 127 exactly? that's the energy? (I understood iete is just and index in eta, but the other one is less obvious)

yes, that's a great suggestion. I modified the numbers to a name constant. This number is the saturation value in the 'split cells'

tvami · 2022-04-29T19:43:24Z

Hi @Michael-Krohn so I see you say no backports, does that mean that this is not relevant for early 13.6 TeV collisions?

cmsbuild · 2022-04-29T21:01:57Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37745/29613

This PR adds an extra 16KB to repository

cmsbuild · 2022-04-29T21:02:20Z

Pull request #37745 was updated. @malbouis, @yuanchao, @epalencia, @cmsbuild, @rekovic, @tvami, @cecilecaillol, @francescobrivio can you please check and sign again.

Michael-Krohn · 2022-04-29T21:11:22Z

Hi @tvami, this will be relevant for early collisions. I didn't realize a backport from 12_4_X was needed for that

cmsbuild · 2022-04-29T21:16:37Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37745/29615

This PR adds an extra 16KB to repository

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37745/29615/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37745/29615/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

tvami · 2022-04-30T00:51:15Z

Hi @Michael-Krohn yes, 900 GeV and low PU 13.6 TeV will use 12_3_X, so after this merged please submit the PR. Now btw you'll have to do 'code-checks'

cmsbuild · 2022-05-02T17:24:11Z

Pull request #37745 was updated. @malbouis, @yuanchao, @epalencia, @cmsbuild, @rekovic, @tvami, @cecilecaillol, @francescobrivio can you please check and sign again.

tvami · 2022-05-02T18:08:21Z

@cmsbuild , please test

tvami · 2022-05-02T18:08:36Z

type hcal

cmsbuild · 2022-05-02T22:05:24Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b114a4/24395/summary.html
COMMIT: 9c241d9
CMSSW: CMSSW_12_4_X_2022-05-02-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37745/24395/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3703092
DQMHistoTests: Total failures: 2
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3703068
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 205 log files, 45 edm output root files, 49 DQM output files
TriggerResults: no differences found

tvami · 2022-05-03T00:56:37Z

Hi @Michael-Krohn @abdoulline I'm surprised to see no differences: https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_4_X_2022-05-02-1100+b114a4/50004/validateJR.html

Should we have tested a specific wf for this?

abdoulline · 2022-05-03T04:58:57Z

@tvami we don't expect to see any difference on 10 ev samples. This is a rare (HE TP saturation) effect.
E.g. "pure" PU (Neutrino + PU) test by our L1 colleagues with 1M ev had no any occurrence.
Also in similar previous PRs (fixes) there were no visible changes in PRs tests #37658 #37471

cecilecaillol · 2022-05-03T08:05:15Z

+l1

tvami · 2022-05-03T12:49:42Z

+alca

tests pass, no changes is expected as explained here modify HCAL output LUT for saturated TPs with |ieta|>=21 #37745 (comment)
@Michael-Krohn please modify the PR title so it includes "HCAL" and please submit the backport PR (and include "CMSSW_12_3_X") after this PR merges

cmsbuild · 2022-05-03T12:50:06Z

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)

perrotta · 2022-05-03T13:10:57Z

+1

cmsbuild added this to the CMSSW_12_4_X milestone Apr 29, 2022

cmsbuild added alca-pending code-checks-pending l1-pending orp-pending pending-signatures tests-pending labels Apr 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 29, 2022

tvami reviewed Apr 29, 2022

View reviewed changes

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 29, 2022

cmsbuild added code-checks-pending and removed code-checks-approved labels Apr 29, 2022

cmsbuild added code-checks-rejected tests-started tests-pending and removed code-checks-pending tests-pending tests-started labels Apr 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels May 2, 2022

cmsbuild added hcal tests-started and removed tests-pending labels May 2, 2022

cmsbuild added tests-approved and removed tests-started labels May 2, 2022

cmsbuild added l1-approved and removed l1-pending labels May 3, 2022

cmsbuild removed alca-pending pending-signatures labels May 3, 2022

cmsbuild added alca-approved fully-signed labels May 3, 2022

cmsbuild added orp-approved and removed orp-pending labels May 3, 2022

cmsbuild merged commit 90892fb into cms-sw:master May 3, 2022

Michael-Krohn changed the title ~~modify output LUT for saturated TPs with |ieta|>=21~~ modify HCAL output LUT for saturated TPs with |ieta|>=21 May 3, 2022

Michael-Krohn mentioned this pull request May 3, 2022

[Backport of] modify HCAL output LUT for saturated TPs with |ieta|>=21 #37789

Merged

This was referenced May 4, 2022

Update vecgeom to 1.1.20 cms-sw/cmsdist#7735

Merged

fix use after scope in DMRChecker::bookResidualsHistogram #37796

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modify HCAL output LUT for saturated TPs with |ieta|>=21 #37745

modify HCAL output LUT for saturated TPs with |ieta|>=21 #37745

Michael-Krohn commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

tvami Apr 29, 2022

Michael-Krohn Apr 29, 2022

tvami commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

Michael-Krohn commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

tvami commented Apr 30, 2022

cmsbuild commented May 2, 2022

tvami commented May 2, 2022

tvami commented May 2, 2022

cmsbuild commented May 2, 2022

tvami commented May 3, 2022

abdoulline commented May 3, 2022 •

edited

Loading

cecilecaillol commented May 3, 2022

tvami commented May 3, 2022 •

edited

Loading

cmsbuild commented May 3, 2022

perrotta commented May 3, 2022

modify HCAL output LUT for saturated TPs with |ieta|>=21 #37745

modify HCAL output LUT for saturated TPs with |ieta|>=21 #37745

Conversation

Michael-Krohn commented Apr 29, 2022

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

tvami Apr 29, 2022

Choose a reason for hiding this comment

Michael-Krohn Apr 29, 2022

Choose a reason for hiding this comment

tvami commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

Michael-Krohn commented Apr 29, 2022

cmsbuild commented Apr 29, 2022

tvami commented Apr 30, 2022

cmsbuild commented May 2, 2022

tvami commented May 2, 2022

tvami commented May 2, 2022

cmsbuild commented May 2, 2022

Comparison Summary

tvami commented May 3, 2022

abdoulline commented May 3, 2022 • edited Loading

cecilecaillol commented May 3, 2022

tvami commented May 3, 2022 • edited Loading

cmsbuild commented May 3, 2022

perrotta commented May 3, 2022

abdoulline commented May 3, 2022 •

edited

Loading

tvami commented May 3, 2022 •

edited

Loading