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

[Backport] Adding the new muon track finder index and three eta cuts features in the GT emulator #41313

Merged

Conversation

elfontan
Copy link
Contributor

@elfontan elfontan commented Apr 11, 2023

PR description:

Backport of #41312.
These updates are needed for the deployment of the Run L1 menu L1Menu_Collisions2023_v1_0_1 in view of the first stable beams at 13.6 TeV or soon after.

This PR includes two updates of the GT emulator accorging to the latest version of the utm library v0.11.2 (see details in CMSLITDPG-1104) and developments of the L1 menu for Run 3:

  • Usage of three eta cuts (up to five cuts are allowed) in the design of the muon algorithms with the upt requirement. The check on the eta range is now implemented as a function checking a vector of eta windows to handle the five cuts in a easier way.
  • New track finder index feature for muons in the uGT emulator needed for the update of the 2023 L1 menu as detailed in CMSLITDPG-1075). Note that the missing muon index data is also included in the GtRecordDumper, needed to produce a test vector as input to the firmware validation. The bits assigned to muon index in the muon object are 36..42 (see Table 6 in the uGT firmware documentation - scales_inputs_2_ugt.pdf). Thank you @arnobaer and Herbert for the work!

PR validation:

Basic tests performed successfully starting from CMSSW_13_0_X_2023-04-06-1100

cmsrel CMSSW_13_0_X_2023-04-06-1100
cd CMSSW_13_0_X_2023-04-06-1100/src
cmsenv
git cms-addpkg L1Trigger/L1TGlobal
git cms-addpkg L1Trigger/Configuration
scram b distclean
git cms-checkdeps -a -A
scram b -j 8
scram b runtests
scram build code-checks
scram build code-format

A cross-check of the countings of each trigger bit in the new firmware and in the updated emulator showed a perfect match.

Basic cmsDriver command for L1Ntuple production successfully tested:

cmsDriver.py l1Ntuple -s RAW2DIGI --python_filename=test.py -n 1000 --no_output --era=Run3 --data --conditions=124X_dataRun3_Prompt_v4 --customise=L1Trigger/Configuration/customiseReEmul.L1TReEmulFromRAW --customise=L1Trigger/L1TNtuples/customiseL1Ntuple.L1NtupleRAWEMU --filein=/store/data/Run2022F/EphemeralZeroBias0/RAW/v1/000/361/468/00000/52351179-2329-47d8-bffc-a01833bb1704.root --fileout=L1Ntuple.root

Note that the temporary release CMSSW_13_0_X_2023-04-06-1100 is not available anymore, so that final tests have been performed in CMSSW_13_0_X_2023-04-15-1100.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 11, 2023

A new Pull Request was created by @elfontan (Elisa Fontanesi) for CMSSW_13_0_X.

It involves the following packages:

  • L1Trigger/L1TGlobal (l1)

@epalencia, @cmsbuild, @cecilecaillol, @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

@francescobrivio
Copy link
Contributor

These updates are needed for the first deployment of the Run L1 menu in preparation of the first stable beams at 13.6 TeV.

Ciao @elfontan according to the latest news we should expect 13.6 TeV SB towards the end of the next week (> 20 April).
Is this PR urgently needed? (I guess only in HLT?)

FYI @tyjyang @fabiocos

@aloeliger
Copy link
Contributor

Backport of #41312

@elfontan
Copy link
Contributor Author

These updates are needed for the first deployment of the Run L1 menu in preparation of the first stable beams at 13.6 TeV.

Ciao @elfontan according to the latest news we should expect 13.6 TeV SB towards the end of the next week (> 20 April). Is this PR urgently needed? (I guess only in HLT?)

FYI @tyjyang @fabiocos

Hi @francescobrivio,
these changes would ensure the support for the deployment of the latest menu that is in preparation (synthesis of the firmware not ready yet). I would like to move the PR to ready for review as soon as we have a confirmation from the firmware about the correct behaviour of the emulator (possibly today).
At that point, I would consider it as urgent in view of the deployment of the new L1 menu, yes. (In HLT, and maybe DQM, I would say.)

Cheers,
--Elisa

@francescobrivio
Copy link
Contributor

Thanks Elisa, makes sense! Please keep us updated on the developments because if we need a new release with this before the 13.6 TeV we don't have mich time! 😄

@aloeliger
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a1dca8/31948/summary.html
COMMIT: 35d2b73
CMSSW: CMSSW_13_0_X_2023-04-12-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41313/31948/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: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3553952
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3553930
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@tyjyang
Copy link

tyjyang commented Apr 13, 2023

Tests for the commit are all passed, are we ready to backport?

@elfontan
Copy link
Contributor Author

elfontan commented Apr 13, 2023

Hello @tyjyang, we are still in the process of validating the behaviour of the current status of the software with the uGT firmware. We would like to wait for this step before moving to the review and integration.
(We are also working on a plan B for the deployment of the new menu in case we are late with the integration for a patch release. This means that we hope to have it done very soon, but it is not highly critical anymore for next week. We will keep you updated.)

Thank you,
--Elisa

@tyjyang
Copy link

tyjyang commented Apr 13, 2023

Thanks Elisa!

@cmsbuild
Copy link
Contributor

Pull request #41313 was updated. @epalencia, @cmsbuild, @cecilecaillol, @aloeliger can you please check and sign again.

@elfontan
Copy link
Contributor Author

Dear all,
after a first round of validation of the GT firmware and emulator behaviour with @arnobaer, the PR has been updated to fix some mismatches in the L1_SingleJet*HE countings caused by the updated checkRangeEta code. While investigating this issue, the code has been improved to implement the check on the eta range as a function that checks a vector of eta windows. In this way, the possibility to have up to five eta cuts is handled in an easier way.

In parallel, an issue with the firmware related to the BMTF muon monitoring seeds based on the usage of the new muon TF index feature has been spotted. We wait for the fix before moving to another round of validation and for now we keep this PR on hold.

We'll keep you posted. Thank you,
--Elisa

@cmsbuild
Copy link
Contributor

Pull request #41313 was updated. @epalencia, @cmsbuild, @cecilecaillol, @aloeliger can you please check and sign again.

@aloeliger
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-a1dca8/32049/summary.html
COMMIT: 1792f9b
CMSSW: CMSSW_13_0_X_2023-04-19-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/41313/32049/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 3 lines to the logs
  • Reco comparison results: 12 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3554298
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3554269
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
  • Checked 213 log files, 164 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

@aloeliger
Copy link
Contributor

+l1

  • last commit removes debug output

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_13_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_13_1_X is complete. 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

perrotta commented Apr 20, 2023

@elfontan updates for L1Trigger/L1TGlobal/plugins/TriggerMenuParser.cc are significantly different here and in the master PR #41312. Could you please explain the differences? If they are actually wanted, please do it also in the PR description.

@elfontan
Copy link
Contributor Author

elfontan commented Apr 21, 2023

Hello @perrotta,
the point is that the TriggerMenuParser was updated in master to use a new logic (as general approach) to read the information from the utm library (PR#41036), but there was the choice to not backport that PR to 13_0_X. @aloeliger can comment more. So I couldn't simply include the changes with a cherry-pick or similar because of the many differences in the basic file. I included the changes manually to reproduce the same results. @aloeliger can probably comment more on this.
Let me know if this clarifies.
Thank you!

Cheers,
Elisa

@aloeliger
Copy link
Contributor

aloeliger commented Apr 21, 2023

@elfontan Are these changes related to the changes I introduced with #41036? If so the changes in that PR were to move from the firmware es types to more CMSSW native utm types and present in CMSSW_13_1_X onwards. Those types should be basically the same however. Where was significant work-around needed for these objects in both PRs?

@aloeliger
Copy link
Contributor

If significant changes in behavior are seen with the es to utm type change, this needs to be addressed immediately.

@elfontan
Copy link
Contributor Author

Hi @aloeliger,
as I answered above, the behaviour is the same as in 13_1_X (= the trigger results when running the emulator in the two different releases to produce test vectors are exactly the same).
I just needed to update "manually" the code of the TriggerMenuParser because the original file was very different between the two releases. I understand that Andrea was referring to this kind of differences.

Elisa

@elfontan
Copy link
Contributor Author

Good morning @perrotta (and @aloeliger),
any follow up here? Was your question referred to the differences explained above?
Thank you in advance.
Cheers,
--Elisa

@aloeliger
Copy link
Contributor

@elfontan I took another look comparing to the original PR. As far as I am concerned these seem reasonable differences to accommodate for changes to the trigger menu parser between CMSSW_13_0 and CMSSW_13_1.

@rappoccio
Copy link
Contributor

+1

Thanks @elfontan, this makes sense.

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.

8 participants