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
Cleanup ECAL topology construction #26466
Conversation
…uction I wonder why the topologies are created on the fly in so many places instead of taking them from EventSetup.
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26466/9298
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages: Calibration/EcalCalibAlgos @perrotta, @cmsbuild, @civanch, @Dr15Jones, @cvuosalo, @tlampen, @christopheralanwest, @ianna, @mdhildreth, @Martin-Grunewald, @franzoni, @tocheng, @slava77, @fwyzard, @pohsun can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+1 |
@slava77 Is my observation
worth to be followed up (starting with an issue)? |
+1 |
I created an issue #26498. |
@Martin-Grunewald this PR is waiting for hlt signature. Do you see any issue with it? |
+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 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) |
+1 |
PR description:
This PR came out as a preparatory cleanup for migrating
Geometry/CaloEventSetup/CaloTopologyBuilder
to EventSetup-consumes andESGetToken
:RecoEcal/EgammaClusterAlgos/PreshowerPhiClusterAlgo
andRecoEcal/EgammaClusterProducers/PreshowerPhiClusterProducer
do not useCaloSubdetectorTopology
so its use is removed from those classesEcalBarrelTopology
andEcalEndcapTopology
directly fromCaloGeometry
instead ofedm::ESHandle<CaloGeometry>
, and remove theCaloGeometry
parameter fromEcalPreshowerTopology
constructor as it was not usedCaloTopology
takes and stores the subdetector topologies asstd::unique_ptr
s.I was surprised how widely spread it is to create the ECAL topology objects on the fly (for each event) instead of asking them from EventSetup.
PR validation:
Tested in CMSSW_10_6_X_2019-04-15-1100 that the code compiles and limited matrix runs. No changes expected.