[L1T] all modules swicthed to lower case and l1t #39244

cecilecaillol · 2022-08-29T20:03:17Z

PR description:

Addressing item 1 of #39194: change L1 modules to start with l1t
Also addressing item 2 of the same issue: Fix hltTriggerSummaryAOD
Also addressing item 5 of the same issue: Verify the PSet cutset in L1TrackSelectionProducerExtended

PR validation:

code-checks, code-format

cmsbuild · 2022-08-29T20:11:06Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39244/31879

This PR adds an extra 216KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File HLTrigger/Configuration/python/HLT_75e33_cff.py modified in PR(s): Development: HLT Phase 2 Tau #39204
- File L1Trigger/L1TTrackMatch/python/L1TrackJetProducer_cfi.py modified in PR(s): [L1T] phase-2, update trackjet simulation #38337

cmsbuild · 2022-08-29T20:11:36Z

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

It involves the following packages:

DQMOffline/L1Trigger (dqm, l1)
HLTrigger/Configuration (hlt)
L1Trigger/Configuration (l1)
L1Trigger/L1CaloTrigger (upgrade, l1)
L1Trigger/L1TTrackMatch (upgrade, l1)
L1Trigger/Phase2L1GMT (l1)
L1Trigger/Phase2L1ParticleFlow (upgrade, l1)
L1Trigger/Phase2L1Taus (l1)
L1Trigger/VertexFinder (l1)

@Martin-Grunewald, @missirol, @epalencia, @emanueleusai, @ahmad3213, @cecilecaillol, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @pmandrik, @micsucmed, @rekovic, @rvenditti can you please review it and eventually sign? Thanks.
@silviodonato, @Martin-Grunewald, @missirol, @trtomei, @beaucero, @rociovilar 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

cecilecaillol · 2022-08-29T20:12:13Z

please test

cmsbuild · 2022-08-30T00:28:17Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d846b7/27178/summary.html
COMMIT: 9a9f1f3
CMSSW: CMSSW_12_6_X_2022-08-29-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/39244/27178/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
ROOTFileChecks: Some differences in event products or their sizes found
Reco comparison results: 23 differences found in the comparisons
DQMHistoTests: Total files compared: 51
DQMHistoTests: Total histograms compared: 3695708
DQMHistoTests: Total failures: 97
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 3695588
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.004 KiB( 50 files compared)
DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
Checked 212 log files, 49 edm output root files, 51 DQM output files
TriggerResults: no differences found

Martin-Grunewald · 2022-08-30T06:44:03Z

A question on the python: I understood that module lables should start with lower-case l1t, while sequences, tasks and paths should start with upper-case L1T?

missirol

@cecilecaillol , thanks for this PR.

I see modules that do not follow the new naming convention. Is this intentional?
Some InputTags seem outdated to me (old names): am I missing something?
Since Item-2 in Remaining issues after the merging of "L1T Phase-2, remove old simulation and adapt DQM (#38442)" #39194 is related to the naming of L1T modules, could you please consider updating HLTrigger/Configuration/python/HLT_75e33/modules/hltTriggerSummaryAOD_cfi.py as in [1]? This would take care of Item-2 without a dedicated PR.

[1]

hltTriggerSummaryAOD = cms.EDProducer( "TriggerSummaryProducerAOD",
    throw = cms.bool( False ),
    processName = cms.string( "*" ),
    moduleLabelPatternsToMatch = cms.vstring( 'hlt*','l1t*' ),
    moduleLabelPatternsToSkip = cms.vstring(  )
)

missirol · 2022-08-30T06:59:19Z

L1Trigger/L1CaloTrigger/python/L1CaloJets_cff.py

        L1TowerCalibrationProducer *
        L1CaloJetProducer *
        L1CaloJetHTTProducer


Why don't these follow the l1t convention?

changed to start with l1t

missirol · 2022-08-30T07:01:28Z

L1Trigger/L1CaloTrigger/python/Phase1L1TJetProducer_cfi.py

@@ -17,7 +17,7 @@
  3.0, 3.083, 3.167, 3.25, 3.333, 3.417, 3.5, 3.583, 3.667, 3.75, 3.833, 3.917, 
  4.0, 4.083, 4.167, 4.25, 4.333, 4.417, 4.5, 4.583, 4.667, 4.75, 4.833, 4.917, 5.0)

-Phase1L1TJetProducer = cms.EDProducer('Phase1L1TJetProducer',
+l1tPhase1JetProducer = cms.EDProducer('Phase1L1TJetProducer',


Shouldn't the name of the cfi be updated accordingly?

The name of the cfi still matches the name of the producer so I would prefer not to change

I thought the cfi should match the module name, not the name of the plugin (for 1 plugin, there could be more than 1 cfi); see also rule 6.13. Anyways, I won't insist on this.

A file named Phase1L1TJetProducer_cfi.py exists already
cfipython/L1Trigger/L1CaloTrigger/Phase1L1TJetProducer_cfi.py
and it will have the same search path as this one: please rename this one l1tPhase1JetProducer_cfi.py. as suggested by @missirol , and correct the import into

L1Trigger/L1CaloTrigger/python/Phase1L1TJets_9x9_cff.py

L1Trigger/L1CaloTrigger/python/Phase1L1TJets_9x9trimmed_cff.py

L1Trigger/L1CaloTrigger/python/Phase1L1TJets_cff.py

(if it is really this one that you want to import, and not the one generated by the fillDescriptions method)

missirol · 2022-08-30T07:04:18Z

L1Trigger/L1TTrackMatch/python/L1TrackerEtMissEmulatorProducer_cfi.py

+    L1TrackInputTag = cms.InputTag("L1TrackSelectionProducer", l1tTrackSelectionProducer.outputCollectionName.value() + "Emulation"),
+    L1TrackAssociatedInputTag = cms.InputTag("L1TrackSelectionProducer", l1tTrackSelectionProducer.outputCollectionName.value() + "AssociatedEmulation"),
    # To bypass GTT input module use  cms.InputTag("TTTracksFromTrackletEmulation", "Level1TTTracks")
    # and set useGTTinput to false
-    L1VertexInputTag = cms.InputTag("VertexProducer", VertexProducer.l1VertexCollectionName.value()),
+    L1VertexInputTag = cms.InputTag("VertexProducer", l1tVertexProducer.l1VertexCollectionName.value()),


These InputTags seem to refer to the old naming (e.g. L1TrackSelectionProducer). Why is that?

Changed to l1tVertexProducer

missirol · 2022-08-30T07:07:24Z

L1Trigger/Phase2L1ParticleFlow/python/L1MetPfProducer_cfi.py

@@ -1,6 +1,6 @@
 import FWCore.ParameterSet.Config as cms

-L1MetPfProducer = cms.EDProducer("L1MetPfProducer",
+l1tMETPFProducer = cms.EDProducer("L1MetPfProducer",
                                 L1PFObjects = cms.InputTag("l1ctLayer1","Puppi"),


Why does l1ctLayer1 not follow the naming convention?

Changed to l1tLayer*

missirol · 2022-08-30T07:08:06Z

L1Trigger/Phase2L1ParticleFlow/python/l1pfJetMet_cff.py

+scPFL1PF            = l1tSeedConePFJetProducer.clone(L1PFObjects = 'l1ctLayer1:PF')
+scPFL1Puppi         = l1tSeedConePFJetProducer.clone()
+scPFL1PuppiEmulator = l1tSeedConePFJetEmulatorProducer.clone(L1PFObject = cms.InputTag('l1ctLayer2Deregionizer:Puppi'))


Why do the scPFL1* objects not follow the new naming convention?

Changed to l1tSC*

missirol · 2022-08-30T07:10:45Z

L1Trigger/Phase2L1ParticleFlow/python/l1pfJetMet_cff.py


-_correctedJets = cms.EDProducer("L1TCorrectedPFJetProducer", 
+correctedJets = cms.EDProducer("L1TCorrectedPFJetProducer", 


The leading underscore should remain in place if correctedJets is not really used in the final configuration (it avoids that correctedJets is imported from this file). Actually, here correctedJets is only used a few lines below to make scPFL1PuppiCorrectedEmulator, so why not just build and customise the latter (without any correctedJets)?

I switched back to _correctedJets

cmsbuild · 2022-08-30T09:56:54Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39244/31886

This PR adds an extra 216KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File HLTrigger/Configuration/python/HLT_75e33_cff.py modified in PR(s): Development: HLT Phase 2 Tau #39204
- File L1Trigger/L1TTrackMatch/python/L1TrackJetProducer_cfi.py modified in PR(s): [L1T] phase-2, update trackjet simulation #38337

cmsbuild · 2022-08-30T09:57:19Z

Pull request #39244 was updated. @Martin-Grunewald, @missirol, @epalencia, @emanueleusai, @ahmad3213, @cecilecaillol, @cmsbuild, @AdrianoDee, @srimanob, @jfernan2, @pmandrik, @micsucmed, @rekovic, @rvenditti can you please check and sign again.

emanueleusai · 2022-09-13T05:40:25Z

+1

re-sign

AdrianoDee · 2022-09-13T07:18:47Z

+upgrade

cecilecaillol · 2022-09-13T07:27:00Z

+l1

mandrenguyen · 2022-09-13T09:03:38Z

+reconstruction

Martin-Grunewald · 2022-09-14T08:43:27Z

+1
resign

perrotta · 2022-09-14T08:56:59Z

+1

cmsbuild · 2022-09-14T08:57:29Z

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 be automatically merged.

srimanob · 2022-09-20T12:12:47Z

Hi,

While making #39450, I see that we have few places that still use ("L1VertexFinderEmulator","l1verticesEmulation") instead of ("l1tVertexFinderEmulator", "l1verticesEmulation"). Is this by intension? Thanks.

https://github.com/cms-sw/cmssw/blob/master/L1Trigger/Phase2L1Taus/plugins/HPSPFTauProducer.cc#L233
https://github.com/cms-sw/cmssw/blob/master/L1Trigger/Configuration/python/SimL1Emulator_cff.py#L170
https://github.com/cms-sw/cmssw/blob/master/L1Trigger/L1TTrackMatch/plugins/L1TrackSelectionProducer.cc#L605

cmsbuild added this to the CMSSW_12_6_X milestone Aug 29, 2022

cmsbuild added code-checks-pending dqm-pending hlt-pending l1-pending orp-pending pending-signatures tests-pending upgrade-pending labels Aug 29, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 29, 2022

cmsbuild added tests-started and removed tests-pending labels Aug 29, 2022

cmsbuild added tests-approved and removed tests-started labels Aug 30, 2022

missirol reviewed Aug 30, 2022

View reviewed changes

cmsbuild added code-checks-pending tests-pending and removed tests-approved code-checks-approved labels Aug 30, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 30, 2022

cmsbuild removed the code-checks-approved label Aug 30, 2022

cmsbuild added dqm-approved and removed dqm-pending labels Sep 13, 2022

cmsbuild added upgrade-approved and removed upgrade-pending labels Sep 13, 2022

cmsbuild added l1-approved and removed l1-pending labels Sep 13, 2022

cmsbuild added reconstruction-approved and removed reconstruction-pending labels Sep 13, 2022

cmsbuild added hlt-approved and removed hlt-pending labels Sep 14, 2022

cmsbuild added fully-signed operations-approved orp-approved and removed operations-pending pending-signatures orp-pending labels Sep 14, 2022

cmsbuild merged commit 366658e into cms-sw:master Sep 14, 2022

This was referenced Sep 14, 2022

[Boost] Update boost 1.80.0 cms-sw/cmsdist#8084

Merged

Retrained network. cms-data/RecoHGCal-TICL#4

Merged

perrotta mentioned this pull request Sep 15, 2022

Remaining issues after the merging of "L1T Phase-2, remove old simulation and adapt DQM (#38442)" #39194

Closed

6 tasks

cecilecaillol mentioned this pull request Sep 20, 2022

[Draft] Update HGCAL pset for Phase-2 HLT menu #39450

Closed

jbsauvan mentioned this pull request Dec 20, 2022

Renaming of modules / sequences / tasks upstream hgc-tpg/cmssw#51

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[L1T] all modules swicthed to lower case and l1t #39244

[L1T] all modules swicthed to lower case and l1t #39244

cecilecaillol commented Aug 29, 2022 •

edited

cmsbuild commented Aug 29, 2022

cmsbuild commented Aug 29, 2022

cecilecaillol commented Aug 29, 2022

cmsbuild commented Aug 30, 2022

Martin-Grunewald commented Aug 30, 2022

missirol left a comment

missirol Aug 30, 2022

cecilecaillol Aug 30, 2022

missirol Aug 30, 2022

cecilecaillol Aug 30, 2022

missirol Sep 1, 2022

perrotta Sep 10, 2022

missirol Aug 30, 2022

cecilecaillol Aug 30, 2022

missirol Aug 30, 2022

cecilecaillol Aug 30, 2022

missirol Aug 30, 2022

cecilecaillol Aug 30, 2022

missirol Aug 30, 2022

cecilecaillol Aug 30, 2022

cmsbuild commented Aug 30, 2022

cmsbuild commented Aug 30, 2022

emanueleusai commented Sep 13, 2022

AdrianoDee commented Sep 13, 2022

cecilecaillol commented Sep 13, 2022

mandrenguyen commented Sep 13, 2022

Martin-Grunewald commented Sep 14, 2022

perrotta commented Sep 14, 2022

cmsbuild commented Sep 14, 2022

srimanob commented Sep 20, 2022


		_correctedJets = cms.EDProducer("L1TCorrectedPFJetProducer",
		correctedJets = cms.EDProducer("L1TCorrectedPFJetProducer",

[L1T] all modules swicthed to lower case and l1t #39244

[L1T] all modules swicthed to lower case and l1t #39244

Conversation

cecilecaillol commented Aug 29, 2022 • edited

PR description:

PR validation:

cmsbuild commented Aug 29, 2022

cmsbuild commented Aug 29, 2022

cecilecaillol commented Aug 29, 2022

cmsbuild commented Aug 30, 2022

Comparison Summary

Martin-Grunewald commented Aug 30, 2022

missirol left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Aug 30, 2022

cmsbuild commented Aug 30, 2022

emanueleusai commented Sep 13, 2022

AdrianoDee commented Sep 13, 2022

cecilecaillol commented Sep 13, 2022

mandrenguyen commented Sep 13, 2022

Martin-Grunewald commented Sep 14, 2022

perrotta commented Sep 14, 2022

cmsbuild commented Sep 14, 2022

srimanob commented Sep 20, 2022

cecilecaillol commented Aug 29, 2022 •

edited