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

Consumes interface for TrackerHitAssociator #8647

Merged
merged 1 commit into from Apr 8, 2015

Conversation

wmtan
Copy link
Contributor

@wmtan wmtan commented Apr 2, 2015

This pull request implements the consumes interface in class TrackerHitAssociator, which involves adding to one existing constructor and adding two new functions, one public. Other very minor optimizations were also made to this class.
This pull request also modifies six modules to use the new TrackerHitAssociator interface so that all products read by the TrackerHitAssociator will be read by getByToken, and therefore registered with the consumes interface. These modules include all five such modules that are known to be used in the relvals.
Note that the current interface to TrackerHitAssociator was left unchanged because many other modules still use it, although comments were added indicating that it is deprecated.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2015

A new Pull Request was created by @wmtan for CMSSW_7_5_X.

Consumes interface for TrackerHitAssociator

It involves the following packages:

SimTracker/TrackerHitAssociation
Validation/GlobalRecHits
Validation/RecoTrack
Validation/TrackerRecHits

@civanch, @nclopezo, @danduggan, @mdhildreth, @cmsbuild, @deguio can you please review it and eventually sign? Thanks.
@makortel, @GiacomoSguazzoni, @rovere, @VinInn, @appeltel, @wmtford, @cerati, @threus, @dgulhan, @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.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@nclopezo you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@wmtan wmtan force-pushed the ConsumesForTrackerHitAssociator branch from da68ea1 to aa04902 Compare April 2, 2015 18:28
@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2015

Pull request #8647 was updated. @civanch, @nclopezo, @danduggan, @mdhildreth, @cmsbuild, @deguio can you please check and sign again.

@davidlange6
Copy link
Contributor

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2015

The tests are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 3, 2015

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 8, 2015

This pull request is fully signed and it will be integrated in one of the next CMSSW_7_5_X IBs unless changes (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @nclopezo, @ktf, @smuzaffar

@davidlange6
Copy link
Contributor

+1

@civanch
Copy link
Contributor

civanch commented Apr 21, 2015

@wmtan , there is a memory leak identified in 750pre2, detailed analysis is in https://hypernews.cern.ch/HyperNews/CMS/get/relval/3565/1/1/1/1/1/1.html

@wmtford
Copy link
Contributor

wmtford commented Apr 22, 2015 via email

@Dr15Jones
Copy link
Contributor

DXR (which doesn't seem to have been well publicized) can find each place where a function is called. In this case the two constructors for TrackerHitAssocciators which don't use the ConsumesCollector:
http://cmslxr.fnal.gov/dxr/CMSSW/search?q=%2Bcallers%3A%22TrackerHitAssociator%3A%3ATrackerHitAssociator%28const+edm%3A%3AEvent+%26%2C+const+edm%3A%3AParameterSet+%26%29%22
http://cmslxr.fnal.gov/dxr/CMSSW/search?q=%2Bcallers%3A%22TrackerHitAssociator%3A%3ATrackerHitAssociator%28const+edm%3A%3AEvent+%26%29%22

@wmtford
Copy link
Contributor

wmtford commented Apr 22, 2015

Hi Chris,

Thanks much. Is there some way to specify the CMSSW release? Some
classes that call the TrackerHitAssociator have been added in 7_5_X.

Bill

On 04/22/2015 12:14 PM, Chris Jones wrote:

DXR (which doesn't seem to have been well publicized) can find each
place where a function is called. In this case the two constructors for
TrackerHitAssocciators which don't use the ConsumesCollector:
http://cmslxr.fnal.gov/dxr/CMSSW/search?q=%2Bcallers%3A%22TrackerHitAssociator%3A%3ATrackerHitAssociator%28const+edm%3A%3AEvent+%26%2C+const+edm%3A%3AParameterSet+%26%29%22
http://cmslxr.fnal.gov/dxr/CMSSW/search?q=%2Bcallers%3A%22TrackerHitAssociator%3A%3ATrackerHitAssociator%28const+edm%3A%3AEvent+%26%29%22


Reply to this email directly or view it on GitHub
#8647 (comment).

@wmtan
Copy link
Contributor Author

wmtan commented Apr 23, 2015

I would be happy to work on cleaning this up after I am back from vacation (May 5).
Don't let this stop anyone else from working on it in the meantime.
Bill Tanenbaum

@Dr15Jones
Copy link
Contributor

DXR was updated to the most recent 7_5_X IB. It is also available from LXR page http://cmslxr.fnal.gov/lxr/index.html

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