Improve performance of Cellular Automaton for regional tracking at HLT #17373

JanFSchulte · 2017-02-01T15:44:51Z

So far, for each tracking region at the HLT, a new CA Graph structure is created. This update intends to improve performance by re-using the data structure and re-filling it for each of the regions. For this, some code is rearranged, but the algorithm itself is not changed at all.

Some details and performance tests can be found here: https://cernbox.cern.ch/index.php/s/UWvE7a7itx6VGBs

The executive summary is:

Checked physics results both offline and online -> 100% identical
Checked timing of modules: Modules using regional tracking speed up between 3 and 40%. The gain strongly depends on the size of the regions, for many small regions the reduction in overhead naturally matters more than for few large ones.

cmsbuild · 2017-02-01T15:45:10Z

A new Pull Request was created by @JanFSchulte (Jan-Frederik Schulte) for CMSSW_9_0_X.

It involves the following packages:

RecoPixelVertexing/PixelTriplets

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

cms-bot commands are listed here #13028

makortel · 2017-02-01T15:48:31Z

@cmsbuild, please test

cmsbuild · 2017-02-01T15:48:46Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17569/console Started: 2017/02/01 19:45

makortel · 2017-02-01T15:50:42Z

Thanks Jan. Could you consider replacing "efficiency" with e.g. "performance" (or "speed") in PR title and description? Just to avoid a possible misconception that physics performance would be altered (which was my first interpretation of "improve efficiency").

JanFSchulte · 2017-02-01T15:52:29Z

Very good point. Fixed.

cmsbuild · 2017-02-01T19:32:43Z

+1
Tested at: 48d82fe
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17373/17569/summary.html

cmsbuild · 2017-02-01T19:32:46Z

Comparison job queued.

cmsbuild · 2017-02-01T21:03:42Z

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

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

20034.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D7_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D7+RecoFullGlobal_2023D7+HARVESTFullGlobal_2023D7
21234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D4_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D4+RecoFullGlobal_2023D4+HARVESTFullGlobal_2023D4
23234.0_TTbar_14TeV+TTbar_14TeV_TuneCUETP8M1_2023D8_GenSimHLBeamSpotFull14+DigiFullTrigger_2023D8+RecoFullGlobal_2023D8+HARVESTFullGlobal_2023D8

JanFSchulte · 2017-02-08T13:05:16Z

Could someone please review this?

I have performed additional performance studies using igprof on a reduced HLT menu running a couple of TkMu paths. Shows 14% improvement for the quadruplet building and 6% for the overall event.

slava77 · 2017-02-08T13:34:59Z

On 2/8/17 5:05 AM, Jan-Frederik Schulte wrote: Could someone please review this? I have performed additional performance studies using igprof on a reduced HLT menu running a couple of TkMu paths. Shows 14% improvement for the quadruplet building and 6% for the overall event.

It's next on my list for review. Expect some feedback today.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#17373 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEdcbtuaaWz1-c5n4sgi7FYfeYNMw-OLks5rab2NgaJpZM4L0BPr>.

cmsbuild · 2017-02-09T20:55:58Z

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

JanFSchulte · 2017-02-09T20:59:48Z

Thanks for reviewing, @slava77 . The new commit should address both your comments.

slava77 · 2017-02-09T21:34:06Z

@cmsbuild please test

cmsbuild · 2017-02-09T21:34:21Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/17721/console Started: 2017/02/09 22:35

cmsbuild · 2017-02-09T22:23:26Z

+1
Tested at: 77f9a41
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-17373/17721/summary.html

cmsbuild · 2017-02-09T22:23:29Z

Comparison job queued.

cmsbuild · 2017-02-09T23:54:56Z

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

slava77 · 2017-02-10T17:54:45Z

I ran 10224 (ttbar PU35 with CA era Run2_2017_trackingPhase1CA)
compared to commit 48d82fe, the latest one has a small increase in CPU

"detachedQuad\|detachedTriplet\|highPtTriplet\|initialStep\|lowPtQuad\|lowPtTriplet" |  grep -v Tracks
   +0.119492      +0.01 %        48.47 ms/ev ->        54.63 ms/ev initialStepHitQuadruplets
   +0.117181      +0.01 %        48.58 ms/ev ->        54.63 ms/ev initialStepHitQuadrupletsPreSplitting
   +0.096095      +0.02 %        76.45 ms/ev ->        84.17 ms/ev detachedQuadStepHitQuadruplets
   +0.085959      +0.03 %       144.01 ms/ev ->       156.95 ms/ev lowPtQuadStepHitQuadruplets
   +0.081517      +0.01 %        29.49 ms/ev ->        31.99 ms/ev highPtTripletStepHitTriplets
   +0.080369      +0.02 %        99.79 ms/ev ->       108.15 ms/ev lowPtTripletStepHitTriplets
   +0.074207      +0.01 %        49.95 ms/ev ->        53.80 ms/ev detachedTripletStepHitTriplets
Todal: 496.74 => 544.32	 delta 47.58

@JanFSchulte please check your HLT tests so that timing improvements remain
and were not blown away by the latest update which is just a one symbol (&) added in for(auto& ntuplet : ntuplets) ntuplet.reserve(localRA_.upper());

slava77 · 2017-02-13T14:10:53Z

@JanFSchulte
did you have a chance to follow up on my comment in #17373 (comment)
?
I'm holding off signing this PR until it's cleared.

JanFSchulte · 2017-02-13T14:16:22Z

@slava77 What I can tell you so far is that I had checked the change in performance using igprof and the numbers for the produce() method did not change. I also asked @makortel for feedback, as he knows the code much better. He suggested a couple of checks that I will run now. Expect a conclusion later today.

slava77 · 2017-02-13T14:25:51Z

+1

for #17373 77f9a41

code changes are in line with the description and the follow up review; the change is not yet affecting the standard workflows
jenkins tests pass and comparisons show no differences
local test with Run2_2017_trackingPhase1CA era shows that there are no changes in results of step3 (step2/HLT is not modified by this era).

@JanFSchulte for me your check with igprof is good enough.

cmsbuild · 2017-02-13T14:26:08Z

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

davidlange6 · 2017-02-14T08:30:29Z

+1

JanFSchulte added 7 commits January 27, 2017 20:00

code cleanup

e737ca1

small fixes

401fea2

a bit more cleanup

e049096

code cleanup

a5a4cdb

code cleanup II

f150018

last cleanup

55922e5

another round of comments from Matti

48d82fe

cmsbuild added this to the Next CMSSW_9_0_X milestone Feb 1, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Feb 1, 2017

cmsbuild added tests-started and removed tests-pending labels Feb 1, 2017

JanFSchulte changed the title ~~Improve efficiency of Cellular Automaton for regional tracking at HLT~~ Improve performance of Cellular Automaton for regional tracking at HLT Feb 1, 2017

cmsbuild added tests-approved and removed tests-started labels Feb 1, 2017

cmsbuild added comparison-available and removed comparison-pending labels Feb 1, 2017

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels Feb 9, 2017

cmsbuild added tests-started and removed tests-pending labels Feb 9, 2017

cmsbuild added tests-approved and removed tests-started labels Feb 9, 2017

cmsbuild added comparison-available and removed comparison-pending labels Feb 9, 2017

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Feb 13, 2017

cmsbuild added orp-approved and removed orp-pending labels Feb 14, 2017

cmsbuild merged commit ad5e08f into cms-sw:CMSSW_9_0_X Feb 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance of Cellular Automaton for regional tracking at HLT #17373

Improve performance of Cellular Automaton for regional tracking at HLT #17373

JanFSchulte commented Feb 1, 2017 •

edited

cmsbuild commented Feb 1, 2017

makortel commented Feb 1, 2017

cmsbuild commented Feb 1, 2017 •

edited

makortel commented Feb 1, 2017

JanFSchulte commented Feb 1, 2017

cmsbuild commented Feb 1, 2017

cmsbuild commented Feb 1, 2017

cmsbuild commented Feb 1, 2017

JanFSchulte commented Feb 8, 2017

slava77 commented Feb 8, 2017 via email

cmsbuild commented Feb 9, 2017

JanFSchulte commented Feb 9, 2017

slava77 commented Feb 9, 2017

cmsbuild commented Feb 9, 2017 •

edited

cmsbuild commented Feb 9, 2017

cmsbuild commented Feb 9, 2017

cmsbuild commented Feb 9, 2017

slava77 commented Feb 10, 2017

slava77 commented Feb 13, 2017

JanFSchulte commented Feb 13, 2017

slava77 commented Feb 13, 2017

cmsbuild commented Feb 13, 2017

davidlange6 commented Feb 14, 2017

Improve performance of Cellular Automaton for regional tracking at HLT #17373

Improve performance of Cellular Automaton for regional tracking at HLT #17373

Conversation

JanFSchulte commented Feb 1, 2017 • edited

cmsbuild commented Feb 1, 2017

makortel commented Feb 1, 2017

cmsbuild commented Feb 1, 2017 • edited

makortel commented Feb 1, 2017

JanFSchulte commented Feb 1, 2017

cmsbuild commented Feb 1, 2017

cmsbuild commented Feb 1, 2017

cmsbuild commented Feb 1, 2017

JanFSchulte commented Feb 8, 2017

slava77 commented Feb 8, 2017 via email

cmsbuild commented Feb 9, 2017

JanFSchulte commented Feb 9, 2017

slava77 commented Feb 9, 2017

cmsbuild commented Feb 9, 2017 • edited

cmsbuild commented Feb 9, 2017

cmsbuild commented Feb 9, 2017

cmsbuild commented Feb 9, 2017

slava77 commented Feb 10, 2017

slava77 commented Feb 13, 2017

JanFSchulte commented Feb 13, 2017

slava77 commented Feb 13, 2017

cmsbuild commented Feb 13, 2017

davidlange6 commented Feb 14, 2017

JanFSchulte commented Feb 1, 2017 •

edited

cmsbuild commented Feb 1, 2017 •

edited

cmsbuild commented Feb 9, 2017 •

edited