Phase 2 GTT MET cos LUT update #42347

NJManganelli · 2023-07-25T07:10:21Z

PR description:

This PR brings the emulation into agreement with the firmware, where a harder cap on the LUT for cos (sin) function lookups (of tracks' phi value entering the L1T MET algorithm) is imposed to meet timing. This reduction in granularity is not expected to produce notable changes in the MET, as the changes merely depopulate the LUT of duplicate values (see https://gitlab.cern.ch/GTT/LibHLS/-/commit/4152f378fc26b06abe6541396c95717ee643ef7c).

This PR corresponds to the gtt/LibHLS PR for the branch here:
https://gitlab.cern.ch/GTT/LibHLS/-/tree/met_cosLUT_update

PR validation:

compiles
scram b code-checks; scram b code-format pass
runs GTT test vector generation successfully, indicating no breaking changes introduced
test vectors generated with this PR match corresponding firmware simulation in the above branch of the HLS repository

This PR will need to be backported to the cms-l1t-offline Phase 2 integration branch

cmsbuild · 2023-07-25T07:16:59Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42347/36369

This PR adds an extra 16KB to repository

cmsbuild · 2023-07-25T07:17:22Z

A new Pull Request was created by @NJManganelli (Nick) for master.

It involves the following packages:

L1Trigger/L1TTrackMatch (upgrade, l1)

@epalencia, @AdrianoDee, @srimanob, @cmsbuild, @aloeliger can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

…e LUTBins constant, ad the LUTShift.

…ction for the MET cos LUT, match the strict dependence on kCosLUTBins constant as in firmware.

…T with truncation of phi value, as in firmware.

cmsbuild · 2023-07-25T20:08:53Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42347/36397

This PR adds an extra 16KB to repository

cmsbuild · 2023-07-25T20:09:13Z

Pull request #42347 was updated. @epalencia, @AdrianoDee, @srimanob, @cmsbuild, @aloeliger can you please check and sign again.

aloeliger · 2023-07-26T11:07:15Z

please test

cmsbuild · 2023-07-26T17:27:20Z

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-57e87d/33909/summary.html
COMMIT: ce6d160
CMSSW: CMSSW_13_3_X_2023-07-26-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42347/33909/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

The relvals timed out after 4 hours.

Comparison Summary

Summary:

You potentially removed 9 lines from the logs
Reco comparison results: 10 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3150117
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3150089
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

NJManganelli · 2023-07-26T18:04:20Z

Are these relval timeouts becoming more frequent?

When it makes sense, please retest

srimanob · 2023-07-26T18:06:53Z

@cmsbuild please test

NJManganelli · 2023-07-26T18:09:31Z

Thanks, Phat!

cmsbuild · 2023-07-26T20:29:29Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-57e87d/33920/summary.html
COMMIT: ce6d160
CMSSW: CMSSW_13_3_X_2023-07-26-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42347/33920/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 10 lines from the logs
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3150117
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3150092
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 207 log files, 159 edm output root files, 48 DQM output files
TriggerResults: no differences found

aloeliger · 2023-07-27T09:33:47Z

+l1

srimanob · 2023-07-27T14:53:26Z

+Upgrade

cmsbuild · 2023-07-27T14:53:49Z

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)

perrotta · 2023-07-27T16:53:17Z

L1Trigger/L1TTrackMatch/plugins/L1TrackerEtMissEmulatorProducer.cc

-              << " Emu Sin(Phi): -" << cosLUT_[globalPhi - phiQuadrants_[3]] << "\n";
+              << " Emu Phi: " << globalPhi
+              << " Emu Cos(Phi): " << cosLUT_[(phiQuadrants_[4] - 1 - globalPhi) >> l1tmetemu::kCosLUTShift]
+              << " Emu Sin(Phi): -" << cosLUT_[(globalPhi - phiQuadrants_[3]) >> l1tmetemu::kCosLUTShift] << "\n";
        }
      } else {


This else is not needed (as it overwrites the default value on the default itself), and it could be removed in order to simplify the code

Indeed, that's a block of code from the department of redundancy department....

If I pass over this code again soon I'll add that bit to the chopping block, otherwise a separate code-clean PR.

Addressed in #42553

perrotta · 2023-07-27T16:53:30Z

+1

cmsbuild added this to the CMSSW_13_3_X milestone Jul 25, 2023

cmsbuild added l1-pending pending-signatures tests-pending orp-pending upgrade-pending code-checks-pending labels Jul 25, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 25, 2023

NJManganelli mentioned this pull request Jul 25, 2023

Phase 2 GTT MET cos LUT update (backport) cms-l1t-offline/cmssw#1154

Merged

Nick Manganelli added 3 commits July 25, 2023 21:59

MET cosLUT update to 1k lookups Atomic 1 -- Add LUTTableSize, move th…

615bfeb

…e LUTBins constant, ad the LUTShift.

MET cosLUT update to 1k lookups Atomic 2 -- Update the generating fun…

ed6cc89

…ction for the MET cos LUT, match the strict dependence on kCosLUTBins constant as in firmware.

MET cosLUT update to 1k lookups Atomic 3 -- update lookups into cosLU…

ce6d160

…T with truncation of phi value, as in firmware.

NJManganelli force-pushed the met_cosLUT_update branch from 1f06fa2 to ce6d160 Compare July 25, 2023 20:00

cmsbuild added code-checks-pending and removed code-checks-approved labels Jul 25, 2023

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 25, 2023

cmsbuild added tests-started and removed tests-pending labels Jul 26, 2023

cmsbuild added tests-rejected and removed tests-started labels Jul 26, 2023

cmsbuild added tests-started and removed tests-rejected labels Jul 26, 2023

cmsbuild added tests-approved and removed tests-started labels Jul 26, 2023

cmsbuild added l1-approved and removed l1-pending labels Jul 27, 2023

cmsbuild added fully-signed upgrade-approved and removed pending-signatures upgrade-pending labels Jul 27, 2023

perrotta reviewed Jul 27, 2023

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Jul 27, 2023

cmsbuild merged commit bbd4719 into cms-sw:master Jul 27, 2023

NJManganelli mentioned this pull request Aug 11, 2023

Phase 2 GTT MET cosLUT overflow fix #42553

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Phase 2 GTT MET cos LUT update #42347

Phase 2 GTT MET cos LUT update #42347

NJManganelli commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

aloeliger commented Jul 26, 2023

cmsbuild commented Jul 26, 2023

NJManganelli commented Jul 26, 2023

srimanob commented Jul 26, 2023

NJManganelli commented Jul 26, 2023

cmsbuild commented Jul 26, 2023

aloeliger commented Jul 27, 2023

srimanob commented Jul 27, 2023

cmsbuild commented Jul 27, 2023

perrotta Jul 27, 2023

NJManganelli Jul 28, 2023

NJManganelli Aug 11, 2023

perrotta commented Jul 27, 2023

Phase 2 GTT MET cos LUT update #42347

Phase 2 GTT MET cos LUT update #42347

Conversation

NJManganelli commented Jul 25, 2023

PR description:

PR validation:

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

cmsbuild commented Jul 25, 2023

aloeliger commented Jul 26, 2023

cmsbuild commented Jul 26, 2023

RelVals-INPUT

Comparison Summary

NJManganelli commented Jul 26, 2023

srimanob commented Jul 26, 2023

NJManganelli commented Jul 26, 2023

cmsbuild commented Jul 26, 2023

Comparison Summary

aloeliger commented Jul 27, 2023

srimanob commented Jul 27, 2023

cmsbuild commented Jul 27, 2023

perrotta Jul 27, 2023

Choose a reason for hiding this comment

NJManganelli Jul 28, 2023

Choose a reason for hiding this comment

NJManganelli Aug 11, 2023

Choose a reason for hiding this comment

perrotta commented Jul 27, 2023