Skip to content

Conversation

@quinnanm
Copy link
Contributor

PR description:

This PR changes the AXOL1TL emulator to fetch the model version from the menu xml rather than the configuration file (as is done currently here [1]). This allows for the model version included in the menu/firmware to be automatically rather than manually set in the emulator to correspond with the correct model version in cms-dist externals [2]. This was proposed to avoid model mismatch errors due to human error.

This PR requires the implementation of utm v0.12.0 [3], and should be merged once that is updated to be the default utm version/configuration. It is not compatible with previous utm versions due to changes in condition types (*if AXOL1TL conditions are included in the menu xml). This PR does not change the default AXOL1TL model version, which is currently v3 [4][5].

PR validation:

Changes were tested with the updated utm using the recipe described here [6]. A testing branch corresponding to this recipe is here [7]. This uses a test xml file for the new utm provided here [8], which is based on this example but with cicada and topo conditions removed [9]. Code-format changes were made, and it passes basic code-checks.

This update is scheduled to be presented in an upcoming Feb 26 L1DPG weekly meeting [10].

Many thanks to Bernhard Arnold and the l1t menu team for their work in adding model versioning capabilities to the utm, and to the L1DPG team and other contributors for their suggestions for this setup.

[1] https://github.com/cms-sw/cmssw/blob/master/L1Trigger/L1TGlobal/python/simGtStage2Digis_cfi.py#L19
[2] https://github.com/cms-hls4ml/AXOL1TL/tags
[3]https://globaltrigger.web.cern.ch/globaltrigger/release/utm/latest_doc/html/releaseNotes/v0_12_0.html
[4] https://github.com/cms-hls4ml/AXOL1TL/releases/tag/v3.0.0
[5] #43791
[6] https://codimd.web.cern.ch/s/q5TiuMfD7
[7]https://github.com/quinnanm/cmssw/tree/emulator_20feb24
[8]https://raw.githubusercontent.com/quinnanm/cmssw/emulator_20feb24/L1Trigger/L1TGlobal/data/Luminosity/startup/L1Menu_Collisions2024_AXO_TOPO_CICADA_v1_0_0_mod.xml
[9] https://cernbox.cern.ch/files/link/public/LeBbgndwFDjWoI5?tiles-size=1&items-per-page=100&view-mode=resource-table
[10] https://indico.cern.ch/category/2091/

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 22, 2024

cms-bot internal usage

@quinnanm quinnanm changed the title Axol1tl emulator modelfrommenupr Get AXOL1TL model version from menu as of utm v0.12.0 Feb 22, 2024
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44054/38995

  • This PR adds an extra 56KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • L1Trigger/L1TGlobal (l1)

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

cms-bot commands are listed here

@Martin-Grunewald
Copy link
Contributor

assign HLT

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Feb 22, 2024

This affects HLT menus in ConfDb - L1TGlobalProducer - and will required a ConfDb parsing. So this PR (and its 14_0 backport) is time critical. In the meantime, to get this PR in, you need to code a customisation removing the AXOL1TL parameter here:
https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/customizeHLTforCMSSW.py
(See the file for examples).
I assume the plan is to backport to 14_0?

@Martin-Grunewald
Copy link
Contributor

assign hlt

@cmsbuild
Copy link
Contributor

New categories assigned: hlt

@Martin-Grunewald,@mmusich you have been requested to review this Pull request/Issue and eventually sign? Thanks

@mmusich
Copy link
Contributor

mmusich commented Feb 22, 2024

This PR requires the implementation of utm v0.12.0 [3], and should be merged once that is updated to be the default utm version/configuration.

For my edification, is that already the default utm version? how can one check that?

epalencia added a commit to epalencia/cmsdist that referenced this pull request Feb 22, 2024
An updated utm library (version 0.12.0) is available: https://gitlab.cern.ch/cms-l1t-utm/utm/-/tree/utm_0.12.0/

cms-sw/cmssw#44054 needs this UTM to be updated to this version
@epalencia
Copy link
Contributor

This PR requires the implementation of utm v0.12.0 [3], and should be merged once that is updated to be the default utm version/configuration.

For my edification, is that already the default utm version? how can one check that?

The current version is 0.11.2: https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_14_1_X/master/utm.spec

I just open a PR to update it to 0.12.0: cms-sw/cmsdist#9029

Co-authored-by: Matti Kortelainen <matti.kortelainen@cern.ch>
@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44054/39010

  • This PR adds an extra 56KB to repository

@aloeliger
Copy link
Contributor

aloeliger commented Mar 14, 2024

L1 DQM uses fixed defined test versions of the L1 Menu

for my education, can you elaborate on this? what does "fixed" mean?

I'm thinking of the wrong thing. There are specific tests for defined unprescaled algorithms:

unprescaledAlgoShortList = cms.untracked.vstring(
"L1_SingleMu22_BMTF",
"L1_SingleMu22_OMTF",
"L1_SingleMu22_EMTF",
"L1_SingleIsoEG28er1p5",
"L1_SingleIsoEG32er2p5",
"L1_SingleEG40er2p5",
"L1_SingleEG60",
"L1_SingleTau120er2p1",
"L1_SingleJet180",
"L1_ETMHF130",
"L1_HTT360er",
"L1_ETT2000"
),
prescaledAlgoShortList = cms.untracked.vstring(
"L1_FirstCollisionInTrain",
"L1_LastCollisionInTrain",
"L1_IsolatedBunch",
"L1_SingleMu0_BMTF",
"L1_SingleMu0_OMTF",
"L1_SingleMu0_EMTF",
"L1_SingleEG10er2p5",
"L1_SingleEG15er2p5",
"L1_SingleEG26er2p5",
"L1_SingleLooseIsoEG28er1p5",
"L1_SingleJet35",
"L1_SingleJet35er2p5",
"L1_SingleJet35_FWD2p5",
"L1_ETMHF100",
"L1_HTT120er",
"L1_ETT1600"

If I had to guess, my primary culprit would still be the menu change rather than anything this PR introduced (changing what bits mean what, causing emu mismatches). Let me talk to Caterina and see if I can understand what all went into this menu.

@artlbv
Copy link
Contributor

artlbv commented Mar 14, 2024

Also, just to clarify, the DQM mismatches were not due to this PR itself as was essentially shown by the previous PR tests mentioned by Marco.

@aloeliger
Copy link
Contributor

Okay, apparently this was a deeper L1T investigation stemming from the Repack. It's a known problem under investigation at this point.

@mmusich
Copy link
Contributor

mmusich commented Mar 14, 2024

Okay, apparently this was a deeper L1T investigation stemming from the Repack. It's a known problem under investigation at this point.

would someone from the L1T team please open a gitHub issue such that there's trace of the investigations?
Thank you.

@cmsbuild
Copy link
Contributor

REMINDER @antoniovilela, @sextonkennedy, @rappoccio: This PR was tested with #44389, please check if they should be merged together

@perrotta
Copy link
Contributor

+alca

@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. @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)
Notice This PR was tested with additional Pull Request(s), please also merge them if necessary: #44389

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 751e1af into cms-sw:master Mar 14, 2024
@smuzaffar
Copy link
Contributor

@antoniovilela, @rappoccio looks like this also needed cms-sw/cmsdist#9064 . As cmssw PR has been merged so I am merging cmsdist PR too

@antoniovilela
Copy link
Contributor

@antoniovilela, @rappoccio looks like this also needed cms-sw/cmsdist#9064 . As cmssw PR has been merged so I am merging cmsdist PR too

Many thanks Shahzad.
Could you use the team mention (@cms-sw/orp-l2) which is easier to track down. I only saw this now.
Thanks.

@perrotta
Copy link
Contributor

@smuzaffar @antoniovilela @rappoccio I remember a "revert" button that easily allowed to revert PRs... but perhaps it was only available for release managers, and I don't have those superpowers any more.

Following the outcomes of the joint operations meeting of 18th March, could any of you prepare the PRs to revert simultaneously:

(While I don't think we need to revert also cms-sw/cmsdist#9064).

Thank you!

mmusich added a commit to mmusich/cmssw that referenced this pull request Mar 19, 2024
@mmusich
Copy link
Contributor

mmusich commented Mar 19, 2024

Following the outcomes of the joint operations meeting of 18th March, could any of you prepare the PRs to revert simultaneously:

here it is: #44466

@mmusich
Copy link
Contributor

mmusich commented Mar 19, 2024

@perrotta

I remember a "revert" button that easily allowed to revert PRs... but perhaps it was only available for release managers, and I don't have those superpowers any more.

for the record, the button is still there (at least I see it and I am not a release manager), but since there were conflicting commits on the same files I don't think the revert would have worked here in any case.

@perrotta
Copy link
Contributor

for the record, the button is still there (at least I see it and I am not a release manager), but since there were conflicting commits on the same files I don't think the revert would have work here in any case.

Ah, ok: found it. I was looking for it in the wrong place
Thank you @mmusich

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.