Update HLT and GT packer for new HI L1 objects #25064

apana · 2018-10-30T23:44:27Z

Update firmware version in cfi for uGT packer (gtStage2Raw_cfi.py) to pack new HI L1 objects
Update HLT so TriggerFilterObjectWithRefs objects get filled properly for new HI L1 objects

cmsbuild · 2018-10-30T23:46:37Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-10-30T23:50:52Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25064/7058

Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25064/7058/git-diff.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25064/7058/git-diff.patch | patch -p1

You can run scram build code-checks to apply code checks directly

cmsbuild · 2018-10-31T00:16:40Z

The code-checks are being triggered in jenkins.

cmsbuild · 2018-10-31T00:19:40Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-25064/7059

cmsbuild · 2018-10-31T00:20:36Z

A new Pull Request was created by @apana (Leonard Apanasvich) for master.

It involves the following packages:

DataFormats/HLTReco
DataFormats/L1TGlobal
EventFilter/L1TRawToDigi
HLTrigger/HLTfilters

@cmsbuild, @Martin-Grunewald, @nsmith-, @rekovic, @thomreis, @fwyzard can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @rovere, @thomreis this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

Martin-Grunewald · 2018-10-31T05:34:32Z

please test

cmsbuild · 2018-10-31T05:34:49Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/31387/console Started: 2018/10/31 06:35

Martin-Grunewald · 2018-10-31T05:47:24Z

Hi @apana

What is the type of the new variables? Are they (all?) global event variables (such as ET, ETmiss, HT), ie, one value per event, or could there be several objects of each (or some) variable in an event, (such as there could be several muons in an event, for example)?

I am asking because I need to pack this up at HLT level also...

fabiocos · 2018-10-31T08:33:00Z

EventFilter/L1TRawToDigi/python/gtStage2Raw_cfi.py

@@ -12,7 +12,8 @@
    JetInputTag    = cms.InputTag("simCaloStage2Digis"),
    EtSumInputTag  = cms.InputTag("simCaloStage2Digis"),
    FedId = cms.int32(1404),
-    FWId = cms.uint32(0x10A6),  # FW version in GMT with vtx-etrapolation
+    ## FWId = cms.uint32(0x10A6),  # FW version in GMT with vtx-etrapolation
+    FWId = cms.uint32(0x10F2),  # FW version for packing new HI centrality variables


@apana @rekovic sorry but is this change backward compatible? This fragment is used by top level configurations for many purposes, not just HI. In all the other updates in this PR I see just addition of pieces, which I would assume is backward compatible.

FwId has increased, and the new version only adds records to pack, not remove any existing records, so should be backward compatible.

cmsbuild · 2018-10-31T10:14:47Z

+1
Tested at: bbace76
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25064/31387/summary.html

cmsbuild · 2018-10-31T10:14:50Z

Comparison job queued.

cmsbuild · 2018-10-31T12:08:45Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-25064/31387/summary.html

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 0 differences found in the comparisons
DQMHistoTests: Total files compared: 32
DQMHistoTests: Total histograms compared: 2994843
DQMHistoTests: Total failures: 24
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2994622
DQMHistoTests: Total skipped: 197
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 31 files compared)
Checked 134 log files, 14 edm output root files, 32 DQM output files

Martin-Grunewald · 2018-10-31T12:28:34Z

All these new event variables are stored in the already existing class DataFormats/L1Trigger/interface/EtSum.h, and the added enums flags what the actual variable is?

Martin-Grunewald · 2018-10-31T15:20:32Z

Comments:

From the direct comments added by myself to the code changes, it is not clear to me whether some blocks are missing or named differently in different parts of the code changes (why?) in this PR, or if anything else is going on in terms of consistency within this PR
It was reported by Jaebeom Park that the PR (in 103X) solves the problem he raised originally which led to an earlier and this PR, which is great!
As alluded to above, in addition, these new variables should appear in the (HLT/TSG) trigger summary product. This is an additional feature, so this PR can already go in now without waiting for the fix to the trigger summary.

To figure out what is needed for the trigger summary, I need more clarifications, especially on the question:
Are all (10? 12? more?) these new event variables stored in/using the already existing class DataFormats/L1Trigger/interface/EtSum.h, and the added enums are there to flag what the actual meaning of the variable stored is?

rekovic · 2018-10-31T17:00:29Z

Hi @Martin-Grunewald.

I believe that @apana covered all the types of L1Triggers that were missing in DataFormats/HLTReco/interface/TriggerTypeDefs.h , some missing from before and some missing now for 2018 HI, by adding the below. These are then used to label the products (seeds) stored in the HLTL1TSeed filter. I don't think that there is any inconsistency there. I imagine that the problem was not noticed before as none of these types were used in HLT seeding before.

     TriggerL1Centrality   =-102,
    TriggerL1MinBiasHFP0  =-103,
    TriggerL1MinBiasHFM0  =-104,
    TriggerL1MinBiasHFP1  =-105,
    TriggerL1MinBiasHFM1  =-106,
    TriggerL1TotalEtEm    =-107,
    TriggerL1MissingHtHF  =-108,
    TriggerL1TowerCount   =-109,
    TriggerL1AsymEt       =-110,
    TriggerL1AsymHt       =-111,
    TriggerL1AsymEtHF     =-112,
    TriggerL1AsymHtHF     =-113,

In addition, some of these types, but lesser number, were also missing from DataFormats/L1TGlobal/src/GlobalObject.cc (for example TowerCount used in 2017 HI run, was already implemented).

Yes. The HI checked that the HLTL1TSeed with DiMuon+Centrality triggers is now fine.
I agree that the work on TriggerSummary can be postponed. Yes, the added enums are flagging the new types of L1 EtSums.

rekovic · 2018-10-31T17:00:40Z

+1

fabiocos · 2018-10-31T17:02:39Z

For the record, discussing with @Martin-Grunewald the TriggeSummary will have to be update after 10_3_1 (if this has to be done on 1 day time scale). It might imply an update of the event content, to be clarified

fabiocos · 2018-10-31T18:29:38Z

@Martin-Grunewald if you have no doubts on this PR in itself, are you ready to sign it so as we can proceed with the integration in view of 10_3_1/
And of course also the 10_3_X back-port needs to be approved (also for L1 @rekovic )

Martin-Grunewald · 2018-11-01T05:36:53Z

I would have wished @apana to comment also.

Martin-Grunewald · 2018-11-01T05:37:00Z

+1

cmsbuild · 2018-11-01T05:37:11Z

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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

fabiocos · 2018-11-01T08:50:00Z

+1

apana · 2018-11-02T03:18:13Z

Hi Martin,

I'm sorry I didn't reply back. I had thought Vladimir answer was sufficient, but in the future I will reply back to make sure. Do you still have any questions? Also, please let me know if you want me to look into updating the TriggerSummary code.

Martin-Grunewald · 2018-11-02T07:35:13Z

Hi Len,

If you look at my specific commenst on the code changes, you see that I am worried that not all the new variables have been added in all locations, as the number of "blocks" added varies.
I interpret Vladimir's feedback as just an assertion/assumption, but you as code author should actually verify - why do you add differently many things in the different locations? And why under different names?

apana added 4 commits October 30, 2018 17:12

Update config to pack new HI centrality variables

0243a26

Update helper functions to include new HI variables

85c9cf4

Update for new L1 HI variables

0c780ed

Remove unnecessary cout

7ac62b3

cmsbuild added this to the CMSSW_10_4_X milestone Oct 30, 2018

cmsbuild added code-checks-pending comparison-pending hlt-pending l1-pending orp-pending pending-signatures tests-pending labels Oct 30, 2018

apana mentioned this pull request Oct 30, 2018

Update HLT and GT packer for new HI L1 objects (Backport to 10_3_X) #25065

Merged

cmsbuild added code-checks-rejected and removed code-checks-pending labels Oct 30, 2018

Apply fix for code style issue found by code-checks

bbace76

cmsbuild added code-checks-pending and removed code-checks-rejected labels Oct 31, 2018

cmsbuild added code-checks-approved and removed code-checks-pending labels Oct 31, 2018

cmsbuild added tests-started and removed tests-pending labels Oct 31, 2018

fabiocos reviewed Oct 31, 2018

View reviewed changes

fabiocos mentioned this pull request Oct 31, 2018

Features needed for CMSSW_10_3_1 for initial HI data taking #25040

Closed

7 tasks

cmsbuild added tests-approved and removed tests-started labels Oct 31, 2018

cmsbuild added comparison-available and removed comparison-pending labels Oct 31, 2018

cmsbuild added l1-approved and removed l1-pending labels Oct 31, 2018

cmsbuild added fully-signed hlt-approved and removed hlt-pending pending-signatures labels Nov 1, 2018

cmsbuild added orp-approved and removed orp-pending labels Nov 1, 2018

cmsbuild merged commit 11d5121 into cms-sw:master Nov 1, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update HLT and GT packer for new HI L1 objects #25064

Update HLT and GT packer for new HI L1 objects #25064

apana commented Oct 30, 2018

cmsbuild commented Oct 30, 2018

cmsbuild commented Oct 30, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

Martin-Grunewald commented Oct 31, 2018

cmsbuild commented Oct 31, 2018 •

edited

Martin-Grunewald commented Oct 31, 2018

fabiocos Oct 31, 2018

rekovic Oct 31, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

Martin-Grunewald commented Oct 31, 2018 •

edited

Martin-Grunewald commented Oct 31, 2018 •

edited

rekovic commented Oct 31, 2018

rekovic commented Oct 31, 2018

fabiocos commented Oct 31, 2018

fabiocos commented Oct 31, 2018

Martin-Grunewald commented Nov 1, 2018

Martin-Grunewald commented Nov 1, 2018

cmsbuild commented Nov 1, 2018

fabiocos commented Nov 1, 2018

apana commented Nov 2, 2018

Martin-Grunewald commented Nov 2, 2018

Update HLT and GT packer for new HI L1 objects #25064

Update HLT and GT packer for new HI L1 objects #25064

Conversation

apana commented Oct 30, 2018

cmsbuild commented Oct 30, 2018

cmsbuild commented Oct 30, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

Martin-Grunewald commented Oct 31, 2018

cmsbuild commented Oct 31, 2018 • edited

Martin-Grunewald commented Oct 31, 2018

fabiocos Oct 31, 2018

Choose a reason for hiding this comment

rekovic Oct 31, 2018

Choose a reason for hiding this comment

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

cmsbuild commented Oct 31, 2018

Martin-Grunewald commented Oct 31, 2018 • edited

Martin-Grunewald commented Oct 31, 2018 • edited

rekovic commented Oct 31, 2018

rekovic commented Oct 31, 2018

fabiocos commented Oct 31, 2018

fabiocos commented Oct 31, 2018

Martin-Grunewald commented Nov 1, 2018

Martin-Grunewald commented Nov 1, 2018

cmsbuild commented Nov 1, 2018

fabiocos commented Nov 1, 2018

apana commented Nov 2, 2018

Martin-Grunewald commented Nov 2, 2018

cmsbuild commented Oct 31, 2018 •

edited

Martin-Grunewald commented Oct 31, 2018 •

edited

Martin-Grunewald commented Oct 31, 2018 •

edited