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

Const only access to GeometricSearchTracker Geometry #3635

Merged
merged 7 commits into from May 6, 2014

Conversation

Dr15Jones
Copy link
Contributor

Modified the interface to GeometricSearchTracker to only give const access to the underlying geometry objects. This allows us to guarantee proper thread-safe interactions with the class.
All clients of GeometricSearchTracker were updated to be compatible with the new interface.

VinInn and others added 6 commits May 1, 2014 23:13
Converted to be compatible with changes to geometry to only give
const access to objects.
Converted to be compatible with changes to geometry to only give
const access to objects.
When returning or passing a DetLayer as an argument to a member
function use const.
The interface to access DetLayer classes is now always const. This
commit updates the client code to match that change.
@cmsbuild
Copy link
Contributor

cmsbuild commented May 1, 2014

A new Pull Request was created by @Dr15Jones (Chris Jones) for CMSSW_7_1_X.

Const only access to GeometricSearchTracker Geometry

It involves the following packages:

CalibTracker/SiStripHitEfficiency
DQM/SiPixelMonitorTrack
DQM/TrackingMonitor
FastSimulation/TrackerSetup
FastSimulation/TrajectoryManager
RecoEgamma/EgammaElectronAlgos
RecoMuon/GlobalTrackingTools
RecoMuon/Navigation
RecoMuon/TrackerSeedGenerator
RecoParticleFlow/PFSimProducer
RecoPixelVertexing/PixelTriplets
RecoTracker/GeometryESProducer
RecoTracker/SpecialSeedGenerators
RecoTracker/TkDetLayers
RecoTracker/TkHitPairs
RecoTracker/TkMSParametrization
RecoTracker/TkNavigation
RecoTracker/TkSeedingLayers
TrackingTools/DetLayers

@diguida, @giamman, @lveldere, @danduggan, @rovere, @cmsbuild, @anton-a, @thspeer, @rcastello, @deguio, @slava77, @Degano, @ojeda, @nclopezo can you please review it and eventually sign? Thanks.
@ghellwig, @GiacomoSguazzoni, @rovere, @lgray, @gpetruc, @cerati, @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.
@nclopezo, @ktf you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@@ -37,7 +37,7 @@ using namespace std;
/// Constructor
MuonNavigationSchool::MuonNavigationSchool(const MuonDetLayerGeometry * muonLayout, bool enableRPC ) : theMuonDetLayerGeometry(muonLayout) {

theAllDetLayersInSystem=&muonLayout->allLayers();
theAllDetLayersInSystem=reinterpret_cast<const std::vector<const DetLayer*>*>(&muonLayout->allLayers());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is needed temporarily until we also update the MuonDetLayerGeometry. The problem is the code assumes MuonDetLayerGeometry and the base class agree on the type. However MuonDetLayerGeometry is using a std::vector<DetLayer*> while the base class is now using std::vector<const DetLayer*>. The two have the same binary layout so the conversion, although strange, is OK.

Changed the interface to MuonDetLayerGeometry to only give const
access to the data it holds. This is needed to enforce thread-safety.
All clients were updated to adapt to the new interface.

This change is part of the GeometricSearchTracker change since
MuonNavigationSchool assumes both geometry providing classes have
compatible interfaces.
@Dr15Jones
Copy link
Contributor Author

It turned out that updating MuonDetLayerGeometry wasn't that hard so I did it as well. The two changes are linked by MuonNavigationSchool and touch many of the same files. Therefore it is
easier to just do them together.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2014

Pull request #3635 was updated. @diguida, @giamman, @lveldere, @danduggan, @rovere, @cmsbuild, @anton-a, @thspeer, @rcastello, @deguio, @slava77, @Degano, @ojeda, @nclopezo can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2014

@cmsbuild
Copy link
Contributor

cmsbuild commented May 2, 2014

@lveldere
Copy link
Contributor

lveldere commented May 5, 2014

+1

@anton-a
Copy link

anton-a commented May 6, 2014

I'll run some longer tests just because of the number of modifications.

@anton-a
Copy link

anton-a commented May 6, 2014

+1

@diguida
Copy link
Contributor

diguida commented May 6, 2014

+1

@cmsbuild
Copy link
Contributor

cmsbuild commented May 6, 2014

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_1_X IBs unless changes (tests are also fine). @nclopezo, @ktf can you please take care of it?

ktf added a commit that referenced this pull request May 6, 2014
Geometry -- Const only access to GeometricSearchTracker Geometry
@ktf ktf merged commit df87489 into cms-sw:CMSSW_7_1_X May 6, 2014
@Dr15Jones Dr15Jones deleted the GeomConstInterface branch May 9, 2014 17:42
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

8 participants