Speed Up Cellular Automaton implementation #18002

VinInn · 2017-03-20T19:53:50Z

Most purely technical changes that produces no regression and speedup CA by about a factor 2
We took the opportunity to fix a small imperfection in the use of the radius the now is w/r/t the beam spot. The effect is mostly to sharpen the pt cut. This change produces minor regressions.

Timing for PU200 ttbar phase2

900
grep TimeRe seedCA.log | cut -c 35-120 | sort -n -r | grep StepHit | head -n2
     0.363026  initialStepHitQuadruplets
     0.067059  initialStepHitDoublets
this PR
grep TimeRe seedCAVN7.log | cut -c 35-120 | sort -n -r | grep StepHit | head -n2
     0.199053  initialStepHitQuadruplets
     0.058190  initialStepHitDoublets

Timing at PU50 PhaseI

900
grep TimeRe caOri.log | cut -c 35-120 | sort -n -r | grep QuadStepHit | head -n4
     0.139890  lowPtQuadStepHitQuadruplets
     0.072600  detachedQuadStepHitQuadruplets
     0.012884  lowPtQuadStepHitDoublets
     0.009104  detachedQuadStepHitDoublets

This PR
grep TimeRe caVecet3.log  | cut -c 35-120 | sort -n -r | grep QuadStepHit | head -n4
     0.094063  lowPtQuadStepHitQuadruplets
     0.045189  detachedQuadStepHitQuadruplets
     0.012195  lowPtQuadStepHitDoublets
     0.008520  detachedQuadStepHitDoublets

MTV for ttbar PU0
http://innocent.home.cern.ch/innocent/RelVal/vectCA3/plots_highPurity/effandfake1.pdf
where blue is 900, red is technical only and black is this PR

cmsbuild · 2017-03-20T19:54:07Z

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

It involves the following packages:

RecoPixelVertexing/PixelTriplets
RecoTracker/TkHitPairs

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

cms-bot commands are listed here #13028

VinInn · 2017-03-20T19:55:10Z

@cmsbuild, please test

cmsbuild · 2017-03-20T19:55:24Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18576/console Started: 2017/03/20 21:36

cmsbuild · 2017-03-20T23:00:51Z

-1

Tested at: 8f311a0

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18002/18576/summary.html

I found follow errors while testing this PR

Failed tests: RelVals AddOn

RelVals:

When I ran the RelVals I found an error in the following worklfows:
10042.0 step2

runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step2_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log

10824.0 step2

runTheMatrix-results/10824.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+ALCAFull_2018+HARVESTFull_2018/step2_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2018_GenSimFull+DigiFull_2018+RecoFull_2018+ALCAFull_2018+HARVESTFull_2018.log

AddOn:

I found errors in the following addon tests:

cmsbuild · 2017-03-20T23:00:53Z

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

VinInn · 2017-03-21T09:06:20Z

I hardly believe that this PR is the cause of the test failure
Would have been much more useful to have a list of the failed AddOn

davidlange6 · 2017-03-21T09:10:36Z

yea, but if you solve everything noted in the comment, you'll likely be ok - two 2017/2018 workflows failed.. 10042 and 10824 Module: CAHitQuadrupletEDProducer (crashed) https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18002/18576/runTheMatrix-results/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step2_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.log certainly related..

…

On Mar 21, 2017, at 10:06 AM, Vincenzo Innocente ***@***.***> wrote: I hardly believe that this PR is the cause of the test failure Would have been much more useful to have a list of the failed AddOn — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

VinInn · 2017-03-21T09:16:06Z

Interesting, as I have run on three different datasets and never crashed

makortel · 2017-03-21T09:19:08Z

The segfault is in HLT, could there be some special case that is not treated properly now?

VinInn · 2017-03-21T09:21:28Z

maybe HLT takes a different path? (there is code that is copy/paste four times...)
Will known soon

cmsbuild · 2017-03-21T12:54:37Z

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

VinInn · 2017-03-21T13:00:06Z

Not obvious what is doing in HLT, we need to check in details.
It does not seem to do what pretended by design (building a single CA for all regions).
For next PR: for the time being just reproduce previous behavior...

VinInn · 2017-03-21T13:00:27Z

@cmsbuild, please test

cmsbuild · 2017-03-21T13:00:43Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/18586/console Started: 2017/03/21 14:01

VinInn · 2017-03-21T13:04:59Z

RecoPixelVertexing/PixelTriplets/plugins/CACell.h

+
+  CACell(const HitDoublets* doublets, int doubletId, const int innerHitId, const int outerHitId) :
+    theDoublets(doublets), theDoubletId(doubletId)
+    ,theInnerR(doublets->rv(doubletId, HitDoublets::inner)) 


this is different that original (now w/r/t/ beamspot)

cmsbuild · 2017-03-21T14:24:20Z

+1
Tested at: 80b67be
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18002/18586/summary.html

cmsbuild · 2017-03-21T14:24:23Z

Comparison job queued.

cmsbuild · 2017-03-21T17:48:14Z

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

slava77 · 2017-03-30T15:32:05Z

+1

for #18002 80b67be

code change is in line with the description; affecting 2017/2018 setups now where CA is enabled
jenkins tests pass and show small differences in 2017/2018 workflows
local test with more events confirm little change in MTV plots and essentially no effect on downstream quantities
speedup is evident, the target reference workflow 2017 ttbar PU35 (10224 ) shows overall less than 1% speedup with significant decrease in CA modules (consistent with what was posted in the PR description)

step2:
2.18 ms/ev ->         0.80 ms/ev hltIter2PFlowPixelHitTriplets
36.89 ms/ev ->        21.37 ms/ev hltPixelTracksHitQuadruplets
10.01 ms/ev ->         5.91 ms/ev hltIter1PFlowPixelHitQuadruplets

step3:
75.90 ms/ev ->        48.32 ms/ev detachedQuadStepHitQuadruplets
51.65 ms/ev ->        33.70 ms/ev initialStepHitQuadruplets
159.75 ms/ev ->       107.67 ms/ev lowPtQuadStepHitQuadruplets
49.14 ms/ev ->        34.04 ms/ev detachedTripletStepHitTriplets
51.97 ms/ev ->        36.43 ms/ev initialStepHitQuadrupletsPreSplitting
103.96 ms/ev ->        77.96 ms/ev lowPtTripletStepHitTriplets
32.08 ms/ev ->        24.19 ms/ev highPtTripletStepHitTriplets
Total in reco: total: 15817 -> 15762	-0.4%
   *Step* modules: 7416 -> 7308        -1.5%

cmsbuild · 2017-03-30T15:32:21Z

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

davidlange6 · 2017-03-30T15:35:46Z

+1

VinInn added 7 commits March 15, 2017 15:38

vectorizing CA

d9970b4

fix r, use relative one

8462b51

vectorize triplets as well

916190e

slimmer,falser

6c5942a

SOA

1e04929

cleanup

a1aa4f3

use a pair, not home made

8f311a0

cmsbuild added this to the Next CMSSW_9_1_X milestone Mar 20, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Mar 20, 2017

cmsbuild added tests-started and removed tests-pending labels Mar 20, 2017

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Mar 20, 2017

fix symptoms of HLT crash

80b67be

cmsbuild added comparison-pending tests-pending and removed comparison-notrun tests-rejected labels Mar 21, 2017

cmsbuild added tests-started and removed tests-pending labels Mar 21, 2017

VinInn commented Mar 21, 2017

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Mar 21, 2017

cmsbuild added comparison-available and removed comparison-pending labels Mar 21, 2017

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Mar 30, 2017

cmsbuild added orp-approved and removed orp-pending labels Mar 30, 2017

cmsbuild merged commit fa37d50 into cms-sw:master Mar 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed Up Cellular Automaton implementation #18002

Speed Up Cellular Automaton implementation #18002

VinInn commented Mar 20, 2017

cmsbuild commented Mar 20, 2017

VinInn commented Mar 20, 2017

cmsbuild commented Mar 20, 2017 •

edited

cmsbuild commented Mar 20, 2017

cmsbuild commented Mar 20, 2017

VinInn commented Mar 21, 2017

davidlange6 commented Mar 21, 2017 via email

VinInn commented Mar 21, 2017

makortel commented Mar 21, 2017

VinInn commented Mar 21, 2017

cmsbuild commented Mar 21, 2017

VinInn commented Mar 21, 2017

VinInn commented Mar 21, 2017

cmsbuild commented Mar 21, 2017 •

edited

VinInn Mar 21, 2017

cmsbuild commented Mar 21, 2017

cmsbuild commented Mar 21, 2017

cmsbuild commented Mar 21, 2017

slava77 commented Mar 30, 2017

cmsbuild commented Mar 30, 2017

davidlange6 commented Mar 30, 2017

Speed Up Cellular Automaton implementation #18002

Speed Up Cellular Automaton implementation #18002

Conversation

VinInn commented Mar 20, 2017

cmsbuild commented Mar 20, 2017

VinInn commented Mar 20, 2017

cmsbuild commented Mar 20, 2017 • edited

cmsbuild commented Mar 20, 2017

cmsbuild commented Mar 20, 2017

VinInn commented Mar 21, 2017

davidlange6 commented Mar 21, 2017 via email

VinInn commented Mar 21, 2017

makortel commented Mar 21, 2017

VinInn commented Mar 21, 2017

cmsbuild commented Mar 21, 2017

VinInn commented Mar 21, 2017

VinInn commented Mar 21, 2017

cmsbuild commented Mar 21, 2017 • edited

VinInn Mar 21, 2017

Choose a reason for hiding this comment

cmsbuild commented Mar 21, 2017

cmsbuild commented Mar 21, 2017

cmsbuild commented Mar 21, 2017

slava77 commented Mar 30, 2017

cmsbuild commented Mar 30, 2017

davidlange6 commented Mar 30, 2017

cmsbuild commented Mar 20, 2017 •

edited

cmsbuild commented Mar 21, 2017 •

edited