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

Improvements to the Phase I quadruplet pixel seeding by Cellular Automaton #15751

Merged
merged 14 commits into from Sep 14, 2016

Conversation

felicepantaleo
Copy link
Contributor

The Cellular Automaton for quadruplet pixel seeding at HLT in Phase I has been improved.

The new version, evaluates the quadruplets for all the configurations of four layers in a single step, hence improving timing.
A better definition of the cut in the xy-plane improves the fake rejection at PixelTracks step.
A new hardPtCut is introduced, giving the possibility, when evaluating the radius of a triplet in the xy plane, to reject those triplets that would result from a pT lower than a threshold.
The CA is still disabled by default and can be run by customizing the process.

It is referred in the following slides as CA all-in-1.

160902_trackingHLT.pdf

@VinInn @rovere @makortel @mtosi

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

A new Pull Request was created by @felicepantaleo (Felice Pantaleo) for CMSSW_8_1_X.

It involves the following packages:

RecoPixelVertexing/PixelTriplets
RecoTracker/Configuration

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @gpetruc, @dgulhan this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Sep 6, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14991/console

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 6, 2016

@felicepantaleo
Copy link
Contributor Author

Another presentation with more details attached
20160906_Felice_GPUsHLT.pdf

@rovere
Copy link
Contributor

rovere commented Sep 12, 2016

Hi all,
it will be important to have this PR integrated in the coming 810_pre12 release.
If there are obstacles, please let us know asap, so that we can promptly react.


if (areAlignedRZ(innerCell, ptmin, thetaCut) && haveSimilarCurvature(innerCell, region_origin_x, region_origin_y, region_origin_radius, phiCut)) {
void checkAlignmentAndTag(CACell* innerCell, const float ptmin, const float region_origin_x, const float region_origin_y, const float region_origin_radius, const float thetaCut, const float phiCut, const float hardPtCut) {
Copy link
Contributor

Choose a reason for hiding this comment

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

here and below some linebreaks should be added to improve readability

@cmsbuild
Copy link
Contributor

Pull request #15751 was updated. @cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please check and sign again.

@felicepantaleo
Copy link
Contributor Author

Hi @slava77
all the modifications you requested were applied, thanks.

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Sep 13, 2016

@cmsbuild please test

@felicepantaleo
thank you for the quick updates

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 13, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/15109/console

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Sep 13, 2016

+1

for #15751 2559fdc

  • updates to CA quadruplet pixel seeding, not yet used in standard workflows.
  • jenkins tests pass and comparisons with baseline show no difference (relying on the last commit tests)
  • local tests with 10224 (ttbar+PU) with CA enabled (customized with customiseForQuadrupletsByCellularAutomaton.py) show decrease in timing (no detailed analysis made to confirm the location of the change). The "baseline" here is IB+CA enabled
    • initialStep seeding performance did not change significantly (fake and efficiency)
    • detachedQuadStep has about ~x2 less tracks,
    • lowPtQuadStep (runs before detachedQuadStep) has few % increase in the number of seeds and tracks, and about a factor of 4 increase in charge mis-ID

timing for the 6 most-changed modules

diff/mean          diff/jobMean    baseline               PR                      module
   -0.530619      -0.10%       235.40 ms/ev ->       136.68 ms/ev detachedQuadStepSeeds
   -0.526078      -0.02%        57.20 ms/ev ->        33.37 ms/ev detachedQuadStepTracks
   -0.462828      -0.04%       108.04 ms/ev ->        67.43 ms/ev detachedQuadStepTrackCandidates
   -0.250578      -0.03%       113.60 ms/ev ->        88.30 ms/ev initialStepSeedsPreSplitting
   -0.161866      -0.03%       187.90 ms/ev ->       159.76 ms/ev lowPtQuadStepSeeds
   +0.114248      +0.08%       622.86 ms/ev ->       698.33 ms/ev lowPtQuadStepTrackCandidates

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-15751/15109/summary.html

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • 20424.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2023D2_GenSimFull+DigiFull_2023D2+RecoFullGlobal_2023D2+HARVESTFullGlobal_2023D2

@slava77
Copy link
Contributor

slava77 commented Sep 14, 2016

here are a few plots of the effects on the reco tracking with CA enabled (currently it looks like the target for this code is HLT, so, the plots are just FYI, without actions required for this PR). These are based on 10224 with CA enabled using customiseForQuadrupletsByCellularAutomaton (black is the IB CMSSW_8_1_X_2016-09-11-2300 and red is IB+thisPR)

wf10224_detachedquad_eff_v_pt

wf10224_detachedquad_fr_v_pt

wf10224_testca_general_fake_v_eta

@mtosi
Copy link
Contributor

mtosi commented Sep 14, 2016

ciao

for reference
timing w/ the current version of the tracking @HLT for 2017 (which is still
preliminary)
shows the following results
timing
where CA all-in-1 refers to this PR
while CA is the version in CMSSW

performance for pixel tracks are the following
eff_pixeltracks
eff_vs_eta_pixeltracks
fake_vs_pt_pixeltracks
fake_vs_eta_pixeltracks

On Wed, Sep 14, 2016 at 3:11 AM, Slava Krutelyov notifications@github.com
wrote:

here are a few plots of the effects on the reco tracking with CA enabled
(currently it looks like the target for this code is HLT, so, the plots are
just FYI, without actions required for this PR). These are based on 10224
with CA enabled using customiseForQuadrupletsByCellularAutomaton (black
is the IB CMSSW_8_1_X_2016-09-11-2300 and red is IB+thisPR)

[image: wf10224_detachedquad_eff_v_pt]
https://cloud.githubusercontent.com/assets/4676718/18496735/3111ce46-79dd-11e6-9bc1-e180a09556e1.png
[image: wf10224_detachedquad_fr_v_pt]
https://cloud.githubusercontent.com/assets/4676718/18496757/5cd47a74-79dd-11e6-9fe9-c8c4a7cb13ed.png

[image: wf10224_testca_general_fake_v_eta]
https://cloud.githubusercontent.com/assets/4676718/18496767/7412fac6-79dd-11e6-98e3-a1472776f82f.png


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15751 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEt587_G5YGFOGWxKmJNOl0pnHLnFmgCks5qp0m4gaJpZM4J2Cfy
.

@davidlange6
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit fc8aa50 into cms-sw:CMSSW_8_1_X Sep 14, 2016
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