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

Tk cloner #2789

Merged
merged 12 commits into from Mar 13, 2014
Merged

Tk cloner #2789

merged 12 commits into from Mar 13, 2014

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Mar 10, 2014

This PR replaces 2752

From 2752

two technical improvements:
1) use SiStripRecHit2D in matcher instead of TTRH
1a) fix matcher for ARM
2) make PixelCPE output used in a single place
(next step will be to return all in a single call: example already in place in base class, need to be implemented in derived classes)
2) removed all setting of quality in SiPixelHit

more cleanup to come
I stop here because of 1a) and to allow work on PixelCPE

No regression observed.
No regression expected.

new improvements
3) cleanup of Constructors (as requested by @slava77)
4) introduction of a Cloner class to "clone' TkHits with a new track hypothesis

No regression observed.
No regression expected.

Next will be to change all Seeders to use only TkHIts (no TTRK)

@cmsbuild
Copy link
Contributor

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

Tk cloner

It involves the following packages:

CommonTools/Utils
DataFormats/TrackerRecHit2D
DataFormats/TrackingRecHit
RecoLocalTracker/ClusterParameterEstimator
RecoLocalTracker/SiPixelRecHits
RecoLocalTracker/SiStripRecHitConverter
RecoMuon/GlobalTrackingTools
RecoTracker/MeasurementDet
RecoTracker/TrackProducer
RecoTracker/TransientTrackingRecHit
TrackingTools/KalmanUpdators
TrackingTools/TransientTrackingRecHit

@nclopezo, @monttj, @cmsbuild, @anton-a, @thspeer, @slava77, @vadler, @Degano can you please review it and eventually sign? Thanks.
@ghellwig, @GiacomoSguazzoni, @rovere, @gpetruc, @cerati, @bachtis 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.

@cmsbuild
Copy link
Contributor

@VinInn
Copy link
Contributor Author

VinInn commented Mar 11, 2014

as things are not moving fast, I took the opportunity to add one more improvements that recompiles
most of CMSSW
5) reduce size of SiPixelHIt and ProjectedHit
needless to say that those changes are "backward incompatible" for anybody (who?)
relying on reco to read and use Tracker RecHit. This is highly discouraged anyhow since CMSSW_2.

No regression observed.
No regression expected.

@cmsbuild
Copy link
Contributor

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

@diguida
Copy link
Contributor

diguida commented Mar 11, 2014

+1
In the AlCa code, there is only one change to accomodate the new DataFormats/TrackerRecHit2D/interface/ProjectedSiStripRecHit2D.h interface (maybe the call to the "old" member function SiStripRecHit2D ProjectedSiStripRecHit2D::originalHit() constcould be removed at all).

@VinInn
Copy link
Contributor Author

VinInn commented Mar 12, 2014

@diguida etal.
I have done to ProjectedHit what already done to MatchedHits.
see
https://twiki.cern.ch/twiki/bin/viewauth/CMS/RecoPerformanceImprovement#SiStripRecHit2D
and https://twiki.cern.ch/twiki/bin/viewauth/CMS/SiStripMatchedRecHit2DFor520
for more details..

@diguida
Copy link
Contributor

diguida commented Mar 12, 2014

@VinInn
Thanks for the documentation. From a quick view, I'd say we could use the improvements in the calibration client code. Anyhow, I prefer to check in more details.

@VinInn
Copy link
Contributor Author

VinInn commented Mar 12, 2014

@diguida the suggested action is "rm"

On 1 Feb, 2012, at 1:06 AM, Mario Galanti wrote:

together with that one, also the related files should go, so the full list of 
files to remove should be:

CalibTracker/SiStripLorentzAngle/interface/SiStripLAProfileBooker.h
CalibTracker/SiStripLorentzAngle/plugins/SiStripLAProfileBooker.cc
CalibTracker/SiStripLorentzAngle/python/SiStripLAProfileBooker_cfi.py
CalibTracker/SiStripLorentzAngle/test/SiStripLAProfileBooker.py

Moreover, the references from the deleted files should be removed from:

CalibTracker/SiStripLorentzAngle/plugins/modules.cc
CalibTracker/SiStripLorentzAngle/test/job_scripts/template.py

@diguida
Copy link
Contributor

diguida commented Mar 12, 2014

@VinInn
Thanks for finding this in your mails! In the twiki it was not mentioned.
If you want, I can make a PR removing those files.

@slava77
Copy link
Contributor

slava77 commented Mar 12, 2014

looking at it

originalHit.omniCluster()),
theOriginalDet(originalHit.det()) {
// std::cout << getRTTI() << ' ' << originalHit.rawId() << ' ' << idet.geographicalId().rawId() << ' ' << originalId() << std::endl;
assert(originalId()==originalHit.rawId());
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really needed? or is it temporary until rework is done?

@VinInn
Copy link
Contributor Author

VinInn commented Mar 13, 2014

Thanks @slava77 for your comments. We will discuss among tracker's developer and will take action in a future PR

@slava77
Copy link
Contributor

slava77 commented Mar 13, 2014

+1

for #2789 6e155a4
no differences as expected

@cmsbuild
Copy link
Contributor

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?

nclopezo added a commit that referenced this pull request Mar 13, 2014
@nclopezo nclopezo merged commit 0a48402 into cms-sw:CMSSW_7_1_X Mar 13, 2014
@VinInn VinInn deleted the TkCloner branch April 21, 2017 11:47
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