L1T ZDC and uGT Emulator to CMSSW_13_3_X

[cfmcginn]

PR description:

PR for L1Trigger ZDC emulation, for upcoming heavy ions datataking. L1Trigger/L1TZDC pkg added containing L1TZDCProducer which puts ZDC based EtSum into the event, which can be picked up in trigger emulation and cut on as with any other EtSum object. New EtSum IDs are added for ZDCP and ZDCM (plus and minus). Additional PR from @elfontan interfacing this PR w/ the Global trigger has since been incorporate here (PR #42635 , now closed; some of the detail in that PR is now incorporated here as of 2023.08.29). This includes an update of the uGT emulator for new ZDC triggers included in the Heavy Ion menu for the 2023 data-taking. The implementation relies on new developments included in the utm libraries v0_11_0.

The main changes are the following:

• The L1Trigger/L1TZDC area containing plugins L1TZDCProducer and L1TZDCAnalyzer, former for producing etSums and the latter primarily used in validation of Produce in comparisons w/ etSums checked offline
• The TriggerMenuParser is adapted to parse the new ZDCPlus and ZDCMinus conditionTypes from the xml menu. A dedicated parsing function parseZdcEnergySum is included to relate these objects to the new ZdcEnergySumTemplate.
• The GlobalBoard is updated to receive a new category of objects similar to the EtSum from the CaloLayer2 coming from the new ZDC Producer included in
• L1T ZDC and uGT Emulator to CMSSW_13_3_X #42634.
• A dedicated ZdcEnergySumTemplate and a ZdcEnergySumCondition class are developed for checking the EtSum threshold on the ZDC objects.
• Addition of condZdcEnergySum to avoid conflicts with existing condEnergySum
• The ste of input tags to the L1TGlobalProducer is updated in simGtStage2Digis_cfi.py.

The following L1Ntuples workflows are also updated to include the new input tags for ZDC objects:

• L1NtupleEMU_cff.py
• L1NtupleRAW_cff.py
• l1UpgradeTree_cfi.py

PR validation:

Emulator output validated against offline ZDC Et Sum calculations ( @hjbossi ) :
ZDCEmulationValidation.pdf

The following suite of tests were done after successful build

scram b runtests
scram build code-checks
scram build code-format

Note two tests failed - see output of attached file:
scramBRunTests_Attempt2.txt

Note that on inspecting full test log, this is because file could not be accessed (do not believe this is an issue w/ voms or certificate, Attempt2 in file name is after rerunning voms). If an appropriate file can be provided, happy to recheck these tests. The full test log is attached here:
testing.log

Other than scram b run tests, other commands completed seemingly correctly - output can be provided if necessary

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

We intend to backport to CMSSW_13_2_X as that is the release designated for heavy ions datataking

Tagging a few additional users to watch this PR: @ginnocen @icali

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42634/36689

• This PR adds an extra 292KB to repository

• Found files with invalid states:

  • L1Trigger/L1TZDC/plugins/L1TStage2ZDCProducer.cc:
    • Added: 1ec1544
    • Modified: 9f8a8e7
    • Deleted: ff9d59e

[cmsbuild]

A new Pull Request was created by @cfmcginn (Chris) for master.

It involves the following packages:

• DataFormats/L1TGlobal (l1)
• DataFormats/L1Trigger (l1)
• L1Trigger/L1TCalorimeter (l1)
• L1Trigger/L1TNtuples (l1)
• L1Trigger/L1TZDC (****)

The following packages do not have a category, yet:

L1Trigger/L1TZDC
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

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

cms-bot commands are listed here

[cfmcginn]

@fwyzard thank you for the initial review - I have made a new commit which I believe resolves most if not all of what you flagged and also went ahead and tried to fix similar issues in other places not yet flagged. I will go thru the comments individually as well, but wanted to ask briefly on how we should proceed.

The PR parallel to this one for the GT (found here: #42635 ) is running into issues in build tests because it has dependencies on this branch that weren't incorporated - we think the best approach is to make a new PR (with all the changes above incorporated) but merging in @elfontan 's branch, so such dependencies are resolved and hopefully there are no build test issues. I am working on testing this merger now in parallel of this work. Do you think this approach makes sense? And if yes, should we close this and the other PR and continue review once the new one is up?

Thanks for your help and let us know

[cmsbuild]

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42634/36707

• This PR adds an extra 304KB to repository

• Found files with invalid states:

  • L1Trigger/L1TZDC/plugins/L1TStage2ZDCProducer.cc:
    • Added: 1ec1544
    • Modified: 9f8a8e7
    • Deleted: ff9d59e

• There are other open Pull requests which might conflict with changes you have proposed:

  • File DataFormats/L1TGlobal/interface/GlobalObject.h modified in PR(s): Updates of the uGT emulator and L1Ntuple workflows for ZDC signals #42635
  • File DataFormats/L1TGlobal/src/GlobalObject.cc modified in PR(s): Updates of the uGT emulator and L1Ntuple workflows for ZDC signals #42635
  • File DataFormats/L1Trigger/interface/EtSum.h modified in PR(s): Updates of the uGT emulator and L1Ntuple workflows for ZDC signals #42635

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-42634/36707/code-format.patch
  e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42634/36707/code-format.patch | patch -p1
  You can also run scram build code-format to apply code format directly

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42634/36708

• This PR adds an extra 80KB to repository

• Found files with invalid states:

  • L1Trigger/L1TZDC/plugins/L1TStage2ZDCProducer.cc:
    • Added: 1ec1544
    • Modified: 9f8a8e7
    • Deleted: ff9d59e

• There are other open Pull requests which might conflict with changes you have proposed:

  • File DataFormats/L1TGlobal/interface/GlobalObject.h modified in PR(s): Updates of the uGT emulator and L1Ntuple workflows for ZDC signals #42635
  • File DataFormats/L1TGlobal/src/GlobalObject.cc modified in PR(s): Updates of the uGT emulator and L1Ntuple workflows for ZDC signals #42635
  • File DataFormats/L1Trigger/interface/EtSum.h modified in PR(s): Updates of the uGT emulator and L1Ntuple workflows for ZDC signals #42635

[cmsbuild]

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

[cmsbuild]

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. @rappoccio, @antoniovilela, @sextonkennedy (and backports should be raised in the release meeting by the corresponding L2)

[fwyzard]

Sorry, one last pedantic comment... could you update the title of the PR to something more descriptive, like "L1T ZDC and uGT emulator" ?

[cfmcginn]

Title is updated - there were a number of flagged things during review to be revisited in near-future PRs as the ZDC team from heavy ions takes over ownership - I want to flag one additional item. The L1TStage2Layer2Producer has additional code here interfacing the caloParamsHelper (for picking up the LUT's) w/ o2o

https://github.com/cfmcginn/cmssw/blob/master/L1Trigger/L1TCalorimeter/plugins/L1TStage2Layer2Producer.cc#L268

L1TZDCProducer does not have similar o2o infrastructure:

https://github.com/cfmcginn/cmssw/blob/l1tZDCEmulator_CMSSW13_3_X/L1Trigger/L1TZDC/plugins/L1TZDCProducer.cc#L238

I asked a bit offline but wasn't able to get a response at this time, so flagging for those future PR.

At this time, should I go to backport? I have a branch I believe ready in
https://github.com/cfmcginn/cmssw/tree/l1tZDCEmulator_CMSSW_13_2_X
and
https://github.com/cfmcginn/cmssw/tree/l1tZDCEmulator_CMSSW_13_2_0_pre3
the latter being for running trigger studies in 13_2 according to existing recipes

[aloeliger]

At this time, should I go to backport? I have a branch I believe ready in
https://github.com/cfmcginn/cmssw/tree/l1tZDCEmulator_CMSSW_13_2_X
and
https://github.com/cfmcginn/cmssw/tree/l1tZDCEmulator_CMSSW_13_2_0_pre3
the latter being for running trigger studies in 13_2 according to existing recipes

@cfmcginn Please wait just a bit on the backport, one of the software release experts may have additional comments. They wait to weigh in until the other groups have had their say. Once this is fully approved and merged, that's the time for backports.

[cfmcginn]

At this time, should I go to backport? I have a branch I believe ready in
https://github.com/cfmcginn/cmssw/tree/l1tZDCEmulator_CMSSW_13_2_X
and
https://github.com/cfmcginn/cmssw/tree/l1tZDCEmulator_CMSSW_13_2_0_pre3
the latter being for running trigger studies in 13_2 according to existing recipes

@cfmcginn Please wait just a bit on the backport, one of the software release experts may have additional comments. They wait to weigh in until the other groups have had their say. Once this is fully approved and merged, that's the time for backports.

Understood, thank you for the clarification.

[rappoccio]

+1

Please prepare the backport PRs ASAP, this is extremely late for datataking. Such a large PR should really have come before the 13_2 release was closed.

[cfmcginn]

+1

Please prepare the backport PRs ASAP, this is extremely late for datataking. Such a large PR should really have come before the 13_2 release was closed.

Backport is up:
#42706