Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[L1T] Phase-2, Create GTT to GT test vectors, and test vectors from the L1TrackSelectionProducer outputs #39422

Merged
merged 13 commits into from Oct 24, 2022

Conversation

cecilecaillol
Copy link
Contributor

PR description:

This PR adds to the DemonstratorTools package the ability to write test vectors for the GTT to GT links. This is in addition to the existing files which contain the TF to GTT inputs, the converted track words created by the GTT common framework, and the GTT to Correlator Layer 1 link. Following the interface document, there are 4 links in total represented in these new files. The links contain words for jets/sums, hadronic taus, light mesons, and isolated tracks/vertices. Some of the formats specified in the interface document have not been written yet. In those cases, 64-bit packets willed with 0s were added in place of the words. In order to check that the 64-bit hexadecimal packets written to the file were correct, a few more branches were added to the L1TrackObjectsNtuple. Local PR: cms-l1t-offline#1042

It also adds to the DemonstratorTools package the ability to write test vectors for outputs from the L1TrackSelectionProducer module. This emulates two firmware modules, one which filters tracks based on the track parameters and another which filters them based on their distance to the primary vertex. These two new files will be created in addition to all the existing test vectors. The format for these files matches the existing track inputs, but with certain track words zeroed out if the tracks are not selected. Local PR: cms-l1t-offline#1043

Right now the L1TrackerEtMissEmulationProducer and the DemonstratorTools package assume a TF to GTT link ordering of the 18 links to have the first 9 be from all of the phi sectors and to care the -eta tracks. The second 9 links would carry the positive eta tracks. However, the GTT firmware and the TF group have been assuming that the -eta and +eta links would be paired, where the first two carry -eta and +eta for sector 0, the second pair does the same for sector 1, and so on. This PR also converts the code to using the latter scheme. Local PR: cms-l1t-offline#1044

PR validation:

From Alexx Perloff: "A set of test vectors was created for the GTT. These test vectors were then compared to the emulator outputs saved in the L1TrackObjectNtuples. I checked by hand that the hexadecimal values saved in the test vectors corresponded to the floating point values saved in the ROOT ntuple (after appropriate un-digitization). I also compared these values to the floating point simulation values in the ROOT ntuple to make sure the simulation and emulation values were close. Everything seems to check out."

The code has also passed the recommended code checks:

scram b code-checks
scram b code-format

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39422/32129

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @cecilecaillol for master.

It involves the following packages:

  • L1Trigger/DemonstratorTools (l1)
  • L1Trigger/L1TTrackMatch (upgrade, l1)

@rekovic, @epalencia, @cmsbuild, @AdrianoDee, @srimanob, @cecilecaillol can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @beaucero, @trtomei 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

@cecilecaillol
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b70e9/27608/summary.html
COMMIT: d98c79e
CMSSW: CMSSW_12_6_X_2022-09-16-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39422/27608/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: 1 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3618332
  • DQMHistoTests: Total failures: 5
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3618305
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@cecilecaillol
Copy link
Contributor Author

+l1

@srimanob
Copy link
Contributor

srimanob commented Oct 5, 2022

@cmsbuild please test

Re-trigger the test after 2.5 weeks. Do we need this PR in 12_5 production?

@epalencia
Copy link
Contributor

Hi @srimanob , no, this one is not needed.

@cmsbuild
Copy link
Contributor

cmsbuild commented Oct 5, 2022

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b70e9/28010/summary.html
COMMIT: d98c79e
CMSSW: CMSSW_12_6_X_2022-10-04-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39422/28010/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
11634.15 step 3
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-9b70e9/41834.0_TTbar_14TeV+2026D94+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal

Summary:

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

@cecilecaillol
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39422/32649

@cmsbuild
Copy link
Contributor

Pull request #39422 was updated. @epalencia, @AdrianoDee, @srimanob, @cecilecaillol, @rekovic can you please check and sign again.

@cecilecaillol
Copy link
Contributor Author

+l1

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-9b70e9/28377/summary.html
COMMIT: d5cd6f1
CMSSW: CMSSW_12_6_X_2022-10-19-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/39422/28377/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: 12 differences found in the comparisons
  • Reco comparison had 6 failed jobs
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3391158
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3391124
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 201 log files, 48 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@AdrianoDee
Copy link
Contributor

+upgrade
(a resign after further review)

@cmsbuild
Copy link
Contributor

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
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants