-
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
Update CaloTowers for hcal flexible segmentation in Phase 2 #3187
Conversation
A new Pull Request was created by @kpedro88 (Kevin Pedro) for CMSSW_6_2_X_SLHC. Update CaloTowers for hcal flexible segmentation in Phase 2 It involves the following packages: CondTools/Geometry @perrotta, @cmsbuild, @apfeiffer1, @Dr15Jones, @lveldere, @civanch, @fwyzard, @ianna, @mdhildreth, @Martin-Grunewald, @anton-a, @thspeer, @giamman, @slava77, @ggovi, @Degano, @ktf, @nclopezo can you please review it and eventually sign? Thanks. |
@kpedro88 - I'm getting this error
for several tests. Is there something new about the CaloTowerConstituentsMapBuilder? Has it been removed or renamed?
|
It hasn't been removed or renamed. I added that line because the map file is not correct for Phase 2. It could be wrong; I got it from the edmConfigBrowser. Maybe the CaloTowerConstituentsMapBuilder is actually subordinate to some other process? |
Pull request #3187 was updated. @perrotta, @cmsbuild, @apfeiffer1, @Dr15Jones, @lveldere, @civanch, @fwyzard, @ianna, @mdhildreth, @Martin-Grunewald, @anton-a, @thspeer, @giamman, @slava77, @ggovi, @Degano, @ktf, @nclopezo can you please check and sign again. |
process.towerMakerPF.HcalPhase = cms.int32(2) | ||
process.towerMakerWithHO.HcalPhase = cms.int32(2) | ||
|
||
process.CaloTowerConstituentsMapBuilder.MapFile = cms.untracked.string("") |
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.
This module doesn't exist until RECO. You should wrap this line in an appropriate test.
Pull request #3187 was updated. @perrotta, @cmsbuild, @apfeiffer1, @Dr15Jones, @lveldere, @civanch, @fwyzard, @ianna, @mdhildreth, @Martin-Grunewald, @anton-a, @thspeer, @giamman, @slava77, @ggovi, @Degano, @ktf, @nclopezo can you please check and sign again. |
+1 |
I realized just now that my previous commit (fix process dependence) had pushed a version of HCalCustoms from before I rebased the branch, so I undid it and pushed a correct commit. |
Pull request #3187 was updated. @perrotta, @cmsbuild, @apfeiffer1, @Dr15Jones, @lveldere, @civanch, @fwyzard, @ianna, @mdhildreth, @Martin-Grunewald, @anton-a, @thspeer, @giamman, @slava77, @ggovi, @Degano, @ktf, @nclopezo can you please check and sign again. |
merge Tests 10000, 10200, 10400, 11200, 11400, 12000, 12800, 13000 and 13600 pass all steps. Tests 12200 (Extended2023HGCalMuon), 14200 (Extended2023HGCal) and 14400 (Extended2023HGCalMuon4Eta) still fail in step 2, but now with a different error:
|
Update CaloTowers for hcal flexible segmentation in Phase 2
I understand the source of this error: the typical geometry configuration imports Geometry.CaloEventSetup.CaloGeometry_cff, which knows about CaloTowerTopology_cfi. In the HGCal geometries, the imports usually in CaloGeometry_cff are instead done one-by-one because of changes to which calorimeters are included. To fix these errors, this line can be added to GeometryExtended2023HGCalReco_cff and any other HGCal reco geometries: |
Upgrade CaloTowers:
flexible ieta, iphi, depth values
ieta values are continuous; may not match HCAL ieta (Phase 2 HCAL ieta is not always continuous between subdetectors)
topology and geometry stored in ES records, dependent on HcalTopology
EE cells mapped to HE towers using getClosestCell() from geometry for Phase 2 HCAL (preliminary method)
This is a rather large pull request.