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

Tracker geom det #4947

Merged
merged 6 commits into from Aug 21, 2014
Merged

Tracker geom det #4947

merged 6 commits into from Aug 21, 2014

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Aug 14, 2014

Following the presentation at the ALCA meeting about APE for muons
https://indico.cern.ch/event/294021/contribution/1/material/slides/0.pdf
It became clear that the GeomDet inheritance tree shall be split between Muon and Tracker at root level.
This requires the coalescence of GeomDetUnit into GeomDet and the introduction of two root classes
TrackerGeomDet and MuonGeomDet.
This PR prepares the infrastructure and implements the Tracker components, leaving to the Muon POG to take care of their specific parts (and if necessary to generalize and extend some of the APE components).

It is quite obvious at this point that the Tracking Geometry infrastructure, as designed in the early days, is not able anymore to support new requirements (it hardly supported alignment requirements even when designed).
Most probably a general clean-up would be very welcome. Unfortunately the amount of code to modify is, as usual, enormous and residing mostly in non critical areas. So I suspect we have just to live with it.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @VinInn (Vincenzo Innocente) for CMSSW_7_2_X.

Tracker geom det

It involves the following packages:

Alignment/CommonAlignment
Alignment/ReferenceTrajectories
Alignment/TrackerAlignment
CalibTracker/SiStripESProducers
CalibTracker/SiStripLorentzAngle
DataFormats/TrackerRecHit2D
DataFormats/TrackingRecHit
FastSimulation/TrackingRecHitProducer
FastSimulation/TrajectoryManager
Fireworks/Geometry
Geometry/CSCGeometry
Geometry/CommonDetUnit
Geometry/DTGeometry
Geometry/GEMGeometry
Geometry/RPCGeometry
Geometry/TrackerGeometryBuilder
RecoLocalTracker/SiStripRecHitConverter
RecoMuon/TransientTrackingRecHit
RecoTracker/DebugTools
RecoTracker/MeasurementDet
RecoTracker/TransientTrackingRecHit
TrackingTools/TransientTrackingRecHit

@civanch, @diguida, @StoyanStoynev, @lveldere, @ianna, @mdhildreth, @cmsbuild, @alja, @Dr15Jones, @rcastello, @cerminar, @slava77, @Degano, @ktf, @nclopezo can you please review it and eventually sign? Thanks.
@ghellwig, @pakhotin, @frmeier, @battibass, @bellan, @trocino, @abbiendi, @GiacomoSguazzoni, @rovere, @alja, @gpetruc, @cerati, @mmusich, @rociovilar, @threus, @bachtis, @venturia this is something you requested to watch as well.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.

@slava77
Copy link
Contributor

slava77 commented Aug 14, 2014

Hi Vincenzo,
are there any changes in results expected?

@VinInn
Copy link
Contributor Author

VinInn commented Aug 14, 2014

No regression expected. No regression observed.
Later on, when Muon will do their part and Muon APE introduced, things may change (inn Muons)...

@slava77
Copy link
Contributor

slava77 commented Aug 15, 2014

uhm, fastsim (5.1 and 50101.0) came out with differences. running them on higher stats to see if it's serious.
I don't see anything obviously relevant in the code though.

@cmsbuild
Copy link
Contributor

-1

Tested at: dcddd70
I ran the usual tests and I found errors in the following unit tests:

---> test runtestUtilAlgos had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4947/618/summary.html

@VinInn
Copy link
Contributor Author

VinInn commented Aug 20, 2014

do not want to apply excessive pressure for the pending signature.
This PR was moved higher in priority on request of Muon POG.
@battibass, @bellan, @trocino, @abbiendi to comment of their schedule requirements

@ianna
Copy link
Contributor

ianna commented Aug 20, 2014

+1

@ktf
Copy link
Contributor

ktf commented Aug 21, 2014

Bypassing AlCa on holidays.

ktf added a commit that referenced this pull request Aug 21, 2014
@ktf ktf merged commit 517a798 into cms-sw:CMSSW_7_2_X Aug 21, 2014
ASvyatkovskiy added a commit to ASvyatkovskiy/cmssw that referenced this pull request Nov 4, 2014
@VinInn VinInn deleted the TrackerGeomDet branch April 21, 2017 11:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants