Skip to content
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

Duplicated configuration of EGammaSuperclusterProducer in the Phase-2 HLT menu #47145

Closed
mmusich opened this issue Jan 20, 2025 · 12 comments · Fixed by #47286
Closed

Duplicated configuration of EGammaSuperclusterProducer in the Phase-2 HLT menu #47145

mmusich opened this issue Jan 20, 2025 · 12 comments · Fixed by #47286

Comments

@mmusich
Copy link
Contributor

mmusich commented Jan 20, 2025

As reported here some duplicates (instances of the same ED module that are configured exactly in the same way down to the last parameter but have different labels) were found in the Phase 2 HLT menu.
Most of them have been addressed at PR #47144, but few remain to be clarified.
All of the remaining ones are related to e/gamma reconstruction:

# EGammaSuperclusterProducer (hltTiclEGammaSuperClusterProducerL1Seeded)
hltTiclEGammaSuperClusterProducerL1Seeded
hltTiclEGammaSuperClusterProducerUnseeded
ticlEGammaSuperClusterProducer

it's not fully clear if these are indeed meant to be duplicates of not, as the naming is contradicting, e.g. “L1Seeded” is a duplicate of “Unseeded”.
Moreover there is another issue concerning the EGammaSuperclusterProducer. One of the instances ticlEGammaSuperClusterProducer used in HLTHgcalTiclPFClusteringForEgammaL1SeededSequence_cfi , is importing things from offline:

from RecoHGCal.TICL.ticlEGammaSuperClusterProducer_cfi import ticlEGammaSuperClusterProducer

This is what we strictly avoided while building the EGamma menu for HLT Phase2 TDR as this is not a desired feature in HLT config.
At the time of HLT TDR, the consensus was NOT importing from offline and having a self-sufficient, independent HLT configuration.
This choice may have changed over time for whatever reason, but can be rediscussed to come to a definitive conclusion and propagate to Phase2 HLT developers.
Also note that there is no concept of L1-seeding for offline. So if HLT starts to import offline config files, then eventually the boundary between unseeded and L1-seeded will be blurred, and HLT-timing will not be benefiting anymore by L1 seeded reconstruction that EGamma had implemented for HLT TDR.

FYI: @rovere @VourMa @swagata87 @RSalvatico

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 20, 2025

cms-bot internal usage

@cmsbuild
Copy link
Contributor

A new Issue was created by @mmusich.

@Dr15Jones, @antoniovilela, @makortel, @mandrenguyen, @rappoccio, @sextonkennedy, @smuzaffar can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@mmusich mmusich changed the title Duplicated configuration of ED modules in the Phase-2 HLT menu Duplicated configuration of EGammaSuperclusterProducer in the Phase-2 HLT menu Jan 20, 2025
@mmusich
Copy link
Contributor Author

mmusich commented Jan 20, 2025

assign hlt

@mmusich
Copy link
Contributor Author

mmusich commented Jan 20, 2025

type egamma

@cmsbuild
Copy link
Contributor

New categories assigned: hlt

@Martin-Grunewald,@mmusich you have been requested to review this Pull request/Issue and eventually sign? Thanks

@makortel
Copy link
Contributor

assign upgrade

@cmsbuild
Copy link
Contributor

New categories assigned: upgrade

@Moanwar,@srimanob,@subirsarkar you have been requested to review this Pull request/Issue and eventually sign? Thanks

@mmusich
Copy link
Contributor Author

mmusich commented Jan 21, 2025

mmusich@daba9d0 fixes the issue and passes runTheMatrix.py --what upgrade -l 29691.203,29691.204.

@mmusich
Copy link
Contributor Author

mmusich commented Jan 21, 2025

tagging also @saumyaphor4252 as discussed in the HLT phase2 upgrade meeting.

@VourMa
Copy link
Contributor

VourMa commented Jan 21, 2025

@rovere and I think it would be better for the EGM POG to take care of this issue. We understand that the duplication arises from the fact that the L1Seeded and Unseeded modules are created based on a procModifier, and this is not what we would expect. As a result, this may not be just a problem of duplication but one of logic in how the procModifier works in this case, and the code may need some refactoring. Our understanding is that the complications came about because of #46010, so it may be useful for @RSalvatico to take a look, if possible?

@mmusich
Copy link
Contributor Author

mmusich commented Feb 6, 2025

After discussion with @rovere and @waredjeb, I proposed a possible solution at #47286

@mmusich
Copy link
Contributor Author

mmusich commented Feb 7, 2025

+hlt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants