add BX info to `GlobalObjectMap` (L1uGT emulation) #47030

missirol · 2024-12-28T16:40:59Z

PR description:

#44940 was solved with a workaround, but it exposed a shortcoming of the L1T-emulation software, namely the fact that the GlobalObjectMap data format does not keep track of the BX values of the L1T objects firing a given L1T algorithm. At HLT, one instance of GlobalObjectMap(Record) is produced by running the emulation of the L1uGT (plugin: L1TGlobalProducer) in order to know the indices of the L1T objects that fired a given L1T algorithm. The main consumers of this information at HLT are the many instances of the HLTL1TSeed plugin, which add these L1T objects to the Event via trigger::TriggerFilterObjectWithRefs. Since GlobalObjectMap does not provide BX info, HLTL1TSeed currently assumes that all these indices refer to L1T objects in BX=0. This assumption is most often correct, because almost all L1T algorithms use only L1T objects assigned to BX=0. On the other hand, L1T algorithms using objects from different BXs do exist (e.g. Cosmics-During-Collisions triggers), and in that case said assumption fails (for more context, see #44940 (comment), CMSHLT-3216 and CMSHLT-3218).

This PR aims to add BX info to the GlobalObjectMap, as a way to fully address the problem described above. The risk/reward ratio of this PR is arguably high: the problem in question has fairly minor consequences in the current HLT menu (only 1 seed is affected), while this PR touches heavily the L1uGT-emulation software, incl. a data format used in RAW data, requiring careful review and validation.

Short description of the changes in this PR.

One data member of the GlobalObjectMap data format (nested vector of ints holding the indices of L1T objects) is replaced with a nested vector of bx:index pairs; backward compatibility with older version of this class is maintained via an iorule in classes_def.xml. The unit test introduced in Add unit test for GlobalObjectMapRecord format #41565 is updated accordingly (if this PR is merged, there will have to be a follow-up PR to add to cms-data a new version of the unit-test input files).
Other classes used in the L1-uGT emulation (e.g. ConditionEvaluation) are updated in order to propagate downstream the BX values of the relevant L1T objects. This is done by changing the current vector of indices to a vector of bx:index pairs (these classes do not correspond to persistent data formats).
HLTL1TSeed is updated to make use of the BX value of the L1T objects, without assuming said BX to be zero. There is one exception to the latter point: in the case where the GlobalObjectMap used by HLTL1TSeed does not contain BX info (which can happen if one reads a GlobalObjectMap object directly from a file written before this PR), HLTL1TSeed will assumes that the BX is 0 (as it does now, in the absence of any alternatives). I do not know of any production workflows that rely on reading a GlobalObjectMap object from older files, so this exception should apply only in very rare cases (if at all).
Fixes mismatches between emulator and fw on CDC dimuon seed #46026 is reverted. While validating this PR, I came to the conclusion that Fixes mismatches between emulator and fw on CDC dimuon seed #46026 should be reverted, see item b) in the validation section below.

PR validation:

addOnTests.py, runTheMatrix.py -l 12834.0 and the unit tests of the affected packages passed.

a) Expected output for the problematic event in #44940. Using [1] with this PR, I see that the module hltL1sCDC adds the expected 2 muons to the Event (one from BX=-1, and one from BX=0); see the printout below, to be compared to the one in #44940 (comment).

--------------------------------------------------
Run             = 381147
LuminosityBlock = 202
Event           = 351398133
--------------------------------------------------

TriggerResults = "TriggerResults" [2 Paths]

   [0] | 0 | HLT_CDC_L2cosmic_10_er1p0_v10
   [1] | 0 | HLTriggerFinalPath

--------------------

TriggerEvent = "hltTriggerSummaryAOD::HLTX" [3 Filters, 188 TriggerObjects]


    [0] hltL1fL1sCDCL1Filtered0::HLTX (2 TriggerObjects)
         [3] FilterId = -81 : pt =    4.000, eta =  0.370, phi =  2.880, id =   0
         [4] FilterId = -81 : pt =    4.000, eta =  0.816, phi = -0.633, id =   0

    [1] hltL1sCDC::HLTX (2 TriggerObjects)
         [3] FilterId = -81 : pt =    4.000, eta =  0.370, phi =  2.880, id =   0
         [4] FilterId = -81 : pt =    4.000, eta =  0.816, phi = -0.633, id =   0

    [2] hltL2fL1sCDCL2CosmicMuL2Filtered3er2stations10er1p0::HLTX (0 TriggerObjects)

b) Revert of #46026: running [1] in CMSSW_15_0_0_pre1, I see that the emulated decision of the CDC seed L1_CDC_SingleMu_3_er1p2_TOP120_DPHI2p618_3p142 from the object-map used at HLT is false and disagrees with the 'firmware' decision in data (which is true). Reverting #46026 fixes this disagreement.

c) Using the test in [2], one can verify that the TriggerObjects in trigger::TriggerEvent are exactly the same before and after this PR, except for the expected difference related to the CDC seed. It's worth noting that this test is limited to the pp menu and 200 input events.

[1] Updated version of the test in #44940 (comment) on 1 event where the CDC seed fired during data-taking.

#!/bin/bash

[ $# -ge 1 ] || exit 1
hltLabel="${1}"

hltGetConfiguration /dev/CMSSW_14_2_0/GRun/V11 \
  --globaltag 141X_dataRun3_HLT_v2 \
  --no-prescale \
  --output minimal \
  --max-events 1 \
  --paths HLT_CDC_L2cosmic_10_er1p0_v*,HLTriggerFinalPath,HLTAnalyzerEndpath \
  --input root://eoscms.cern.ch//eos/cms/store/group/tsg/FOG/error_stream_root/run381147/run381147_ls0202_index000187_fu-c2b05-29-01_pid2159904.root \
  > "${hltLabel}".py

cat <<@EOF >> "${hltLabel}".py
process.options.numberOfThreads = 1
process.options.numberOfStreams = 0

process.source.skipEvents = cms.untracked.uint32( 56 )

del process.MessageLogger
process.load("FWCore.MessageLogger.MessageLogger_cfi")
process.MessageLogger.L1TGlobalSummary = cms.untracked.PSet()

# print the (unprescaled) emulated L1T decisions
process.hltL1TGlobalSummaryEmul = process.hltL1TGlobalSummary.clone(
    AlgInputTag = 'hltGtStage2ObjectMap',
    ExtInputTag = 'hltGtStage2ObjectMap',
)
process.HLTAnalyzerEndpath += process.hltL1TGlobalSummaryEmul
@EOF

cmsRun "${hltLabel}".py &> "${hltLabel}".log
mv output.root "${hltLabel}".root

# script to print the TriggerObjects in an EDM file
wget https://raw.githubusercontent.com/missirol/hltScripts/33d04bc85c5cb1581442ff5465904706ccb9b969/hltTests/hltFWLite_exa01.py -O printTriggerObjects.py
chmod u+x printTriggerObjects.py
./printTriggerObjects.py -n 1 -v 10 -i "${hltLabel}".root > "${hltLabel}"_triggerObjects.txt
rm -f printTriggerObjects.py

[2] Test running the latest HLT pp menu on 200 events of 2024 HLTPhysics data.

#!/bin/bash

[ $# -ge 1 ] || exit 1
hltLabel="${1}"

hltGetConfiguration /dev/CMSSW_14_2_0/GRun/V11 \
  --globaltag 141X_dataRun3_HLT_v2 \
  --no-prescale \
  --output minimal \
  --max-events 200 \
  --input root://eoscms.cern.ch//eos/cms/store/user/cmsbuild/store/data/Run2024I/EphemeralHLTPhysics0/RAW/v1/000/386/593/00000/91a08676-199e-404c-9957-f72772ef1354.root \
  > "${hltLabel}".py

cat <<@EOF >> "${hltLabel}".py
process.options.numberOfThreads = 1
process.options.numberOfStreams = 0
@EOF

cmsRun "${hltLabel}".py &> "${hltLabel}".log
mv output.root "${hltLabel}".root

# script to print the TriggerObjects in an EDM file
wget https://raw.githubusercontent.com/missirol/hltScripts/33d04bc85c5cb1581442ff5465904706ccb9b969/hltTests/hltFWLite_exa01.py -O printTriggerObjects.py
chmod u+x printTriggerObjects.py
./printTriggerObjects.py -n -1 -v 10 -i "${hltLabel}".root > "${hltLabel}"_triggerObjects.txt
rm -f printTriggerObjects.py

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:

N/A

If there is agreement on reverting #46026, that revert should arguably be backported to CMSSW_14_2_X.

This reverts commit c8aefec.

cmsbuild · 2024-12-28T16:41:26Z

cms-bot internal usage

cmsbuild · 2024-12-28T16:44:03Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47030/43147

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/L1TGlobal/test/TestReadGlobalObjectMapRecord.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372
- File DataFormats/L1TGlobal/test/TestWriteGlobalObjectMapRecord.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372

cmsbuild · 2024-12-28T16:44:29Z

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

It involves the following packages:

DataFormats/L1TGlobal (l1)
HLTrigger/HLTfilters (hlt)
L1Trigger/L1TGlobal (l1)

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

cms-bot commands are listed here

cmsbuild · 2024-12-28T21:50:46Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47030/43149

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/L1TGlobal/test/TestReadGlobalObjectMapRecord.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372
- File DataFormats/L1TGlobal/test/TestWriteGlobalObjectMapRecord.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372

cmsbuild · 2024-12-28T21:51:05Z

Pull request #47030 was updated. @Martin-Grunewald, @aloeliger, @cmsbuild, @epalencia, @mmusich can you please check and sign again.

missirol · 2024-12-28T21:51:19Z

The latest push (052da05) contains only some minor adjustments.

missirol · 2024-12-28T17:20:06Z

DataFormats/L1TGlobal/test/TestGlobalObjectMapRecordFormat.sh

+#oldFiles="testGlobalObjectMapRecord_CMSSW_15_0_0_pre2_split_99.root testGlobalObjectMapRecord_CMSSW_15_0_0_pre2_split_0.root"
+#for file in $oldFiles; do
+#  inputfile=$(edmFileInPath DataFormats/L1TGlobal/data/$file) || die "Failure edmFileInPath DataFormats/L1TGlobal/data/$file" $?
+#  cmsRun ${LOCAL_TEST_DIR}/test_readGlobalObjectMapRecord_cfg.py --inputFileName "$inputfile" --globalObjectMapClassVersion 11 || die "Failed to read old file $file" $?
+#done
+


This part could be removed. It is supposed to be un-commented once new files are added to cms-data with the first (pre-)release built after this PR is merged (CMSSW_15_0_0_pre2, if one is optimistic).

missirol · 2024-12-29T07:50:08Z

Moving to draft while I try to implement a couple of improvements [1] [2]. In the meantime, the PR could be tested nonetheless, to see if there are any unexpected problems.

[1] iorule for GlobalObjectMap, to ensure backward-compatibility while moving to a nested vector of bx:index pairs.
[2] Avoid changing the "0" convention for MuonShower and EtSum conditions, and adjust HLTL1TSeed accordingly.

cmsbuild · 2024-12-31T08:52:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47030/43152

There are other open Pull requests which might conflict with changes you have proposed:
- File DataFormats/L1TGlobal/test/TestReadGlobalObjectMapRecord.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372
- File DataFormats/L1TGlobal/test/TestWriteGlobalObjectMapRecord.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372

cmsbuild · 2024-12-31T08:52:56Z

Pull request #47030 was updated.

missirol · 2024-12-31T08:56:03Z

The latest push (ba11d70) contains the following updates.

Simplified the update of the GlobalObjectMap data format, using an iorule to maintain backward compatibility.
Removed any non-technical changes in EnergySumCondition, EnergySumZdcCondition and MuonShowerCondition: these conditions continue to output "index 0" as pre-PR, because (a) I'm not sure of the side-effects of changing this convention, and (b) not changing this also has the advantage that the updated version of HLTL1TSeed remains compatible with GlobalObjectMap(Record) objects stored in older files.
Removed spurious changes (minor unrelated cleanup which was included in the first version of this PR), for easier review.
Minimised changes in downstream classes: class methods are not renamed, only the underlying data type is changed as needed.

The description has been updated accordingly. ba11d70 passes the validation given in the PR description.

'Draft mode' removed. This PR is ready for review (I don't have further updates planned for it at the moment).

mmusich · 2025-01-02T10:57:42Z

@cmsbuild, please test

cmsbuild · 2025-01-02T13:08:49Z

+1

Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4802ec/43611/summary.html
COMMIT: ba11d70
CMSSW: CMSSW_15_0_X_2025-01-02-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/47030/43611/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: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 49
DQMHistoTests: Total histograms compared: 3818730
DQMHistoTests: Total failures: 476
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3818234
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 48 files compared)
Checked 214 log files, 184 edm output root files, 49 DQM output files
TriggerResults: no differences found

missirol · 2025-01-03T08:32:08Z

The DQM bin-by-bin comparisons of wf 140.56 suggest that this PR is reducing the number of L1-uGT data-emulator mismatches of the type "Emulator fired but not Data" for BX=-2 and BX=-1. These mismatches concern the L1T seeds in [1]; these mismatches are present in the baseline (pre-PR), and they are absent post-PR (the L1T menu in question corresponds to the one used in run-327524). This looks like another indication that #46026 should be reverted (as done in this PR).

As for the other workflows, I don't see any DQM bin-by-bin differences that could be related to this PR.

[1]

[Trigger Bit - Algorithm Name]

000 - L1_SingleMuCosmics
002 - L1_SingleMuCosmics_OMTF
003 - L1_SingleMuCosmics_EMTF
004 - L1_SingleMuOpen
005 - L1_SingleMu0_DQ
006 - L1_SingleMu0
009 - L1_SingleMu0_EMTF
010 - L1_SingleMu3
029 - L1_DoubleMu0
030 - L1_DoubleMu0_SQ
240 - L1_DoubleMuOpen_MaxDr3p5

Revert "Fixes mismatches between emulator and fw on CDC dimuon seed"

5db9511

This reverts commit c8aefec.

cmsbuild added this to the CMSSW_15_0_X milestone Dec 28, 2024

cmsbuild added l1-pending hlt-pending pending-signatures tests-pending orp-pending code-checks-pending changes-dataformats labels Dec 28, 2024

missirol mentioned this pull request Dec 28, 2024

Fixes mismatches between emulator and fw on CDC dimuon seed #46026

Merged

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 28, 2024

missirol force-pushed the devel_CMSHLT3218 branch from dbe158b to 052da05 Compare December 28, 2024 21:47

cmsbuild added code-checks-pending and removed code-checks-approved labels Dec 28, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 28, 2024

missirol commented Dec 28, 2024

View reviewed changes

missirol marked this pull request as draft December 29, 2024 07:44

add BX info to GlobalObjectMap (L1uGT emulation)

ba11d70

missirol force-pushed the devel_CMSHLT3218 branch from 052da05 to ba11d70 Compare December 31, 2024 08:49

cmsbuild added code-checks-pending and removed code-checks-approved labels Dec 31, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Dec 31, 2024

missirol marked this pull request as ready for review December 31, 2024 08:56

cmsbuild added tests-started and removed tests-pending labels Jan 2, 2025

cmsbuild added tests-approved and removed tests-started labels Jan 2, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add BX info to `GlobalObjectMap` (L1uGT emulation) #47030

add BX info to `GlobalObjectMap` (L1uGT emulation) #47030

missirol commented Dec 28, 2024 •

edited

Loading

cmsbuild commented Dec 28, 2024 •

edited

Loading

cmsbuild commented Dec 28, 2024

cmsbuild commented Dec 28, 2024

cmsbuild commented Dec 28, 2024

cmsbuild commented Dec 28, 2024

missirol commented Dec 28, 2024

missirol Dec 28, 2024

missirol commented Dec 29, 2024

cmsbuild commented Dec 31, 2024

cmsbuild commented Dec 31, 2024

missirol commented Dec 31, 2024

mmusich commented Jan 2, 2025

cmsbuild commented Jan 2, 2025

missirol commented Jan 3, 2025

add BX info to GlobalObjectMap (L1uGT emulation) #47030

Are you sure you want to change the base?

add BX info to GlobalObjectMap (L1uGT emulation) #47030

Conversation

missirol commented Dec 28, 2024 • edited Loading

PR description:

PR validation:

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:

cmsbuild commented Dec 28, 2024 • edited Loading

cmsbuild commented Dec 28, 2024

cmsbuild commented Dec 28, 2024

cmsbuild commented Dec 28, 2024

cmsbuild commented Dec 28, 2024

missirol commented Dec 28, 2024

missirol Dec 28, 2024

Choose a reason for hiding this comment

missirol commented Dec 29, 2024

cmsbuild commented Dec 31, 2024

cmsbuild commented Dec 31, 2024

missirol commented Dec 31, 2024

mmusich commented Jan 2, 2025

cmsbuild commented Jan 2, 2025

Comparison Summary

missirol commented Jan 3, 2025

add BX info to `GlobalObjectMap` (L1uGT emulation) #47030

add BX info to `GlobalObjectMap` (L1uGT emulation) #47030

missirol commented Dec 28, 2024 •

edited

Loading

cmsbuild commented Dec 28, 2024 •

edited

Loading