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

Phase2-hgx91 Change the signature of CaloCellGeometry #21808

Merged
merged 5 commits into from Jan 17, 2018

Conversation

bsunanda
Copy link
Contributor

@bsunanda bsunanda commented Jan 5, 2018

Use std::shared_ptr for accessing the CaloCellGeometry so that new Cell pointers can be created on demand (useful for HGCal and also Hcal).

This is a copy of PR# 21440 which has a merge conflict and could not be recovered. #21440 will be closed once this PR is merged

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 5, 2018

The code-checks are being triggered in jenkins.

@bsunanda
Copy link
Contributor Author

bsunanda commented Jan 5, 2018

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 5, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 5, 2018

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/25318/console Started: 2018/01/06 00:10

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 5, 2018

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

It involves the following packages:

CalibCalorimetry/EcalTPGTools
Calibration/EcalAlCaRecoProducers
Calibration/EcalCalibAlgos
Calibration/HcalAlCaRecoProducers
Calibration/HcalCalibAlgos
Calibration/IsolatedParticles
Calibration/Tools
CondTools/Geometry
DQM/EcalMonitorTasks
DQMOffline/CalibCalo
DQMOffline/EGamma
DQMOffline/Ecal
DQMOffline/Hcal
DQMOffline/JetMET
DataFormats/ParticleFlowReco
FastSimulation/CaloGeometryTools
Fireworks/Geometry
Geometry/CaloEventSetup
Geometry/CaloGeometry
Geometry/CaloTopology
Geometry/EcalAlgo
Geometry/EcalTestBeam
Geometry/ForwardGeometry
Geometry/HGCalGeometry
Geometry/HcalTowerAlgo
HiggsAnalysis/HiggsToGammaGamma
JetMETCorrections/MinBias
L1Trigger/L1THGCal
RecoBTau/JetCrystalsAssociator
RecoEcal/EgammaClusterAlgos
RecoEcal/EgammaClusterProducers
RecoEcal/EgammaCoreTools
RecoEgamma/EgammaHFProducers
RecoEgamma/EgammaHLTProducers
RecoEgamma/EgammaIsolationAlgos
RecoEgamma/EgammaTools
RecoJets/JetProducers
RecoLocalCalo/CaloTowersCreator
RecoLocalCalo/EcalRecProducers
RecoLocalCalo/HGCalRecAlgos
RecoLocalCalo/HcalRecAlgos
RecoLocalFastTime/FTLCommonAlgos
RecoMET/METAlgorithms
RecoMET/METFilters
RecoParticleFlow/PFClusterProducer
RecoParticleFlow/PFSimProducer
RecoTBCalo/EcalSimpleTBAnalysis
RecoTBCalo/EcalTBAnalysisCoreTools
RecoTauTag/RecoTau
SUSYBSMAnalysis/HSCP
SimCalorimetry/CaloSimAlgos
SimCalorimetry/EcalEBTrigPrimProducers
SimCalorimetry/EcalSimAlgos
SimCalorimetry/EcalTrigPrimProducers
TrackingTools/TrackAssociator
Validation/EcalDigis
Validation/GlobalHits
Validation/GlobalRecHits
Validation/HGCalValidation
Validation/HcalDigis
Validation/HcalHits
Validation/HcalRecHits

@ghellwig, @lveldere, @ianna, @kpedro88, @nsmith-, @rekovic, @thomreis, @vanbesien, @perrotta, @civanch, @monttj, @silviodonato, @cmsbuild, @fwyzard, @Dr15Jones, @mdhildreth, @jfernan2, @cerminar, @slava77, @ggovi, @Martin-Grunewald, @ssekmen, @vazzolini, @kmaeshima, @arunhep, @dmitrijus, @alja, @franzoni, @lpernie can you please review it and eventually sign? Thanks.
@ghellwig, @rappoccio, @gouskos, @yslai, @felicepantaleo, @jainshilpi, @abbiendi, @TaiSakuma, @kpedro88, @argiro, @Martin-Grunewald, @imarches, @pfs, @ahinzmann, @varuns23, @seemasharmafnal, @mmarionncern, @kreczko, @sethzenz, @JyothsnaKomaragiri, @makortel, @acaudron, @jhgoh, @lgray, @mmusich, @jdolen, @HuguesBrun, @drkovalskyi, @ferencek, @trocino, @rociovilar, @vandreev11, @Sam-Harper, @GiacomoSguazzoni, @tocheng, @VinInn, @jdamgov, @bellan, @nhanvtran, @gkasieczka, @rovere, @schoef, @ebrondol, @dgulhan, @cseez, @calderona, @battibass, @edjtscott, @alja, @mverzett, @folguera, @cbernet, @gpetruc, @matt-komm, @mariadalfonso, @amarini, @pvmulder, @bachtis, @jbsauvan this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 6, 2018

@cmsbuild
Copy link
Contributor

cmsbuild commented Jan 6, 2018

Comparison job queued.

@rekovic
Copy link
Contributor

rekovic commented Jan 11, 2018

+1

@civanch
Copy link
Contributor

civanch commented Jan 11, 2018

+1

@bsunanda
Copy link
Contributor Author

@ggovi @dmitrijus @alja Can you approve this please

@bsunanda
Copy link
Contributor Author

@fabiocos Could you please take this PR into the IB and to pre4?

@fabiocos
Copy link
Contributor

Is this PR ok for db and dqm? It is quite complex, we should try to move it forward asap, thanks

@bsunanda
Copy link
Contributor Author

@fabiocos @ggovi @dmitrijus The real change is the use of CaloCellGeometry object. The pointer for this is now a shared_ptr rather than a normal one. The main impact is on geometry and reco code. In other parts some casting styles are changed and a bug in HGCal code is corrected.

@dmitrijus
Copy link
Contributor

+1

@ggovi
Copy link
Contributor

ggovi commented Jan 16, 2018

+1

@fabiocos
Copy link
Contributor

@alja Sorry, but we need to move forward with this PR

@fabiocos
Copy link
Contributor

merge

@cmsbuild cmsbuild merged commit 225ce8a into cms-sw:master Jan 17, 2018
@@ -71,13 +71,16 @@ class FlatTrd : public CaloCellGeometry {

void getTransform( Tr3D& tr, Pt3DVec* lptr ) const override;

void setPosition ( const GlobalPoint& p ) { m_global = p; setRefPoint(p); }
void setBackPoint( const GlobalPoint& p ) { setBackPoint(p); }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong. This will result in infinite recursion until the stack is exhausted.

@bsunanda
Copy link
Contributor Author

bsunanda commented Jan 22, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment