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

Tracking: Switch off cluster shape in step6 seeding #3818

Merged
merged 2 commits into from May 14, 2014

Conversation

thspeer
Copy link
Contributor

@thspeer thspeer commented May 12, 2014

By removing the cluster shape filter in the TobTec step seeding, the efficiency for displaced tracks is increased from ~23 to ~26 %. This recovers the lost efficiency when the CCC was introduced.

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @thspeer for CMSSW_7_1_X.

Tracking: Switch off cluster shape in step6 seeding

It involves the following packages:

RecoPixelVertexing/PixelLowPtUtilities
RecoTracker/IterativeTracking

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

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented May 14, 2014

tobTecStepTrackCandidates time essentially doubles in wf 202.0 (also doubles in 25202, the PU20)
Is this expected/needed?
This is +2% in CPU in these tests.

I can guess that PU40 can go up even more.
Thomas, do you have tests for this?

Should it go to pre8 or should we better iterate on this later in 72X?

@cerati
Copy link
Contributor

cerati commented May 14, 2014

Indeed this is bug fix, as when the strip triplet seeding for tobTec was developed the clusterShapeFilter was intentionally left out not to lose efficiency. It went in with the CCC, but it was not intentional.
In my opinion, with the new seeding and CCC, we have reduced timing enough that we should keep efficiency as high as possible at this stage (the 2x mentioned by slava refers to a versions that is much faster than in 62X).
Maybe Thomas has the proper numbers handy?

@slava77
Copy link
Contributor

slava77 commented May 14, 2014

It would be good to see comparisons of timing, at least as a reference.
My memory is that the last improvements in CCC (thickness-dependent cuts) had about the same effect as this PR on timing, only with a negative side. So, at least these two cancel out.

@thspeer
Copy link
Contributor Author

thspeer commented May 14, 2014

In my tests, the tracking time goes from 11.6 sec/event to 12.8 sec/event. This can be compared to the 16.0 I saw in 710pre4, before the new CCC was added. So we still have a clear improvement in the timing.

@slava77
Copy link
Contributor

slava77 commented May 14, 2014

are these the same inputs?

@thspeer
Copy link
Contributor Author

thspeer commented May 14, 2014

yes, exactly the same events

@slava77
Copy link
Contributor

slava77 commented May 14, 2014

+1

for #3818 a4acb64

tested in CMSSW_7_1_X_2014-05-13-1400 (test area sign375)
some increase in efficiency vs vtx pos without much increase in fake rate

(dijet QCD below)
wf38_cutsrecohp_vsvtx

changes as expected, although some finer tuning of selections may have helped (the number of seeds in iter 6 almost doubles)

@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?

davidlange6 added a commit that referenced this pull request May 14, 2014
Tracking: Switch off cluster shape in step6 seeding
@davidlange6 davidlange6 merged commit 39e8432 into cms-sw:CMSSW_7_1_X May 14, 2014
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

5 participants