-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add hlt
(HLT
) label to Phase-2 HLT modules (sequences)
#45636
add hlt
(HLT
) label to Phase-2 HLT modules (sequences)
#45636
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45636/41148 |
A new Pull Request was created by @mmusich for master. It involves the following packages:
@Martin-Grunewald, @antoniovagnerini, @cmsbuild, @mmusich, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
PFClusters = cms.InputTag("particleFlowClusterHGCalFromTICLUnseeded"), | ||
PFSuperClusterCollectionBarrel = cms.string('particleFlowSuperClusterECALBarrel'), | ||
PFClusters = cms.InputTag("hltParticleFlowClusterHGCalFromTICLUnseeded"), | ||
PFSuperClusterCollectionBarrel = cms.string('hltParticleFlowSuperClusterECALBarrel'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I am missing something, but also according to the other EDProducers, shouldn't this be
PFSuperClusterCollectionBarrel = cms.string('hltParticleFlowSuperClusterECALBarrel'), | |
PFSuperClusterCollectionBarrel = cms.string('particleFlowSuperClusterECALBarrel'), |
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I can tell, I agree with @waredjeb regarding consistency.
On the other hand, this product (I mean the barrel one created by this HGCal-labelled module) is never consumed. So any name would do w/o crashes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank @rovere for the check.
On the other hand, this product (I mean the barrel one created by this HGCal-labelled module) is never consumed. So any name would do w/o crashes.
I would propose to go ahead with the PR as is now to avoid re-triggering a lot of signatures and then fix this immediately after, if people agree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed at #45768. Let me know if you spot other problems.
Hi @mmusich thanks a lot! For what concerns TICL/HGCAL, I think we also need to modify the RecoHGCal Event content https://github.com/cms-sw/cmssw/blob/master/RecoHGCal/Configuration/python/RecoHGCal_EventContent_cff.py adding the new HLT collections to We can make a commit on top of this PR such that you can just cherry-pick it |
Good point! The same holds true for the tracking products . |
+1 Size: This PR adds an extra 220KB to repository Comparison SummarySummary:
|
Hi @mmusich, I think you can use this commit. I've checked that the collections are saved in the event content. Let us know if it works :) |
- add `hlt` to modules - add `HLT` to sequences
24f4756
to
7d91971
Compare
+Upgrade |
+1
|
+hlt
|
+1 |
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. |
…ming fixes leftover from #45636
PR description:
The goal of this PR is to package together the renaming of the HLT module and sequences labels in the HLT phase-2 menu (kindly provided by @AuroraPerego in AuroraPerego@226b557, see discussion at #45500 (comment)) with the necessary updates of the Trigger DQM and Validation sequences configurations consuming said products in order to stay in synch with it (that I provide in mmusich@24f4756)
PR validation:
None, will rely on the bot tests.
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:
This PR is not a backport and it should not be backported.
I open this PR in draft mode as I am not sure that I have caught all the products consumed downstream that need to be renamed in the configuration.
I would like to request feedback to:
Cc: @rovere @SohamBhattacharya @VourMa