Speed up pattern recognition in Pixels by implementing a 5 sigma compatibility window in TkPixelMeasurementDet #18408

VinInn · 2017-04-19T15:28:01Z

implement a 5 sigma compatibility window in TkPixelMeasurementDet
(once cluster has been sorted in x)

produces a speed up of 10% in pattern recognition (for instance initialStepTrackCandidates)
for PU50

expect negligible regressions

The usual suspects: electron, photon, muon, cosmics are left unchanged
see #14467 #12846 for what can happen in messing with those...

cmsbuild · 2017-04-19T15:28:23Z

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

It involves the following packages:

RecoLocalTracker/SiPixelClusterizer
RecoTracker/MeasurementDet

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @makortel, @felicepantaleo, @GiacomoSguazzoni, @rovere, @VinInn, @mschrode, @dkotlins, @gpetruc, @ebrondol, @threus, @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

VinInn · 2017-04-19T15:32:50Z

910_pre2 perf report

   - 6.21% GroupedCkfTrajectoryBuilder::rebuildSeedingRegion                                                                                                                                              ▒
      - 5.05% GroupedCkfTrajectoryBuilder::rebuildSeedingRegion                                                                                                                                           ▒
         - 4.96% GroupedCkfTrajectoryBuilder::groupedLimitedCandidates                                                                                                                                    ▒
            - 4.81% GroupedCkfTrajectoryBuilder::advanceOneLayer                                                                                                                                          ▒
               - 4.17% TrajectorySegmentBuilder::segments                                                                                                                                                 ▒
                  - 3.02% LayerMeasurements::groupedMeasurements                                                                                                                                          ▒
                     + 1.58% TkPixelMeasurementDet::measurements                                                                                                                                          ▒
                     + 0.92% GeometricSearchDet::groupedCompatibleDets                                                                                                                                    ▒
                     + 0.34% TkGluedMeasurementDet::measurements                                                                                                                                          ▒
                  + 1.00% TrajectorySegmentBuilder::addGroup                                                                                                                                              ▒
               + 0.29% BaseCkfTrajectoryBuilder::findStateAndLayers                                                                                                                                       ▒
      + 1.06% GroupedCkfTrajectoryBuilder::backwardFit

and

   - 3.06% TkPixelMeasurementDet::measurements                                                                                                                                                            ▒
      - 2.64% TkPixelMeasurementDet::recHits                                                                                                                                                              ▒
         - 2.49% TkPixelMeasurementDet::buildRecHit                                                                                                                                                       ▒
            - 2.29% PixelCPEBase::getParameters                                                                                                                                                           ▒
               + 1.58% PixelCPEGeneric::localPosition                                                                                                                                                     ▒
                 0.15% _ZNK12PixelCPEBase27computeAnglesFromTrajectoryERKNS_8DetParamERNS_12ClusterParamERK25LocalTrajectoryParameters@plt                                                                ▒
                 0.11% PixelCPEGeneric::localError                                                                                                                                                        ▒
                 0.11% _ZNK12PixelCPEBase9setTheCluERKNS_8DetParamERNS_12ClusterParamE@plt                                                                                                                ▒
                 0.11% _ZdlPvm@plt                                                                                                                                                                        ▒
        0.30% Chi2MeasurementEstimator::estimate

VinInn · 2017-04-19T15:35:20Z

this PR

   - 4.26% GroupedCkfTrajectoryBuilder::rebuildSeedingRegion                                                                                                                                                                            ▒
      - 3.10% GroupedCkfTrajectoryBuilder::rebuildSeedingRegion                                                                                                                                                                         ▒
         - 3.00% GroupedCkfTrajectoryBuilder::groupedLimitedCandidates                                                                                                                                                                  ▒
            - 2.82% GroupedCkfTrajectoryBuilder::advanceOneLayer                                                                                                                                                                        ▒
               - 2.19% TrajectorySegmentBuilder::segments                                                                                                                                                                               ▒
                  - 1.47% LayerMeasurements::groupedMeasurements                                                                                                                                                                        ▒
                     - 0.67% GeometricSearchDet::groupedCompatibleDets                                                                                                                                                                  ▒
                        + 0.31% TBLayer::groupedCompatibleDetsV                                                                                                                                                                         ▒
                        + 0.30% PixelForwardLayerPhase1::groupedCompatibleDetsV                                                                                                                                                         ▒
                     + 0.33% TkPixelMeasurementDet::measurements                                                                                                                                                                        ▒
                     + 0.31% TkGluedMeasurementDet::measurements                                                                                                                                                                        ▒
                  + 0.55% TrajectorySegmentBuilder::addGroup                                                                                                                                                                            ▒
               + 0.26% BaseCkfTrajectoryBuilder::findStateAndLayers                                                                                                                                                                     ▒
      + 1.07% GroupedCkfTrajectoryBuilder::backwardFit

and

   - 0.75% TkPixelMeasurementDet::measurements                                                                                                                                                                                          ▒
      - 0.63% TkPixelMeasurementDet::compHits                                                                                                                                                                                           ▒
         - 0.43% TkPixelMeasurementDet::buildRecHit                                                                                                                                                                                     ▒
            - 0.39% PixelCPEBase::getParameters                                                                                                                                                                                         ▒
               - 0.28% PixelCPEGeneric::localPosition                                                                                                                                                                                   ▒
                    0.10% _ZN15SiPixelGenError4qbinEiffffRfS0_S0_S0_S0_S0_S0_S0_S0_S0_S0_S0_S0_@plt                                                                                                                                     ▒

VinInn · 2017-04-19T15:36:08Z

@cmsbuild , please test

cmsbuild · 2017-04-19T15:36:35Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19264/console Started: 2017/04/19 17:57

cmsbuild · 2017-04-19T17:43:14Z

-1

Tested at: 4aec9c5

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

I found follow errors while testing this PR

Failed tests: RelVals

RelVals:

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

runTheMatrix-results/4.22_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC/step2_RunCosmics2011A+RunCosmics2011A+RECOCOSD+ALCACOSD+SKIMCOSD+HARVESTDC.log

cmsbuild · 2017-04-19T17:43:17Z

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

slava77 · 2017-04-19T17:48:16Z

@cmsbuild please test

the last one was a file read error, possibly transient

cmsbuild · 2017-04-19T17:48:35Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19273/console Started: 2017/04/19 19:49

cmsbuild · 2017-04-19T19:10:40Z

+1
Tested at: 4aec9c5
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18408/19273/summary.html

perrotta · 2017-05-02T07:49:32Z

RecoTracker/MeasurementDet/plugins/TkPixelMeasurementDet.cc

+  yl = 5.f*std::sqrt(yl);
+
+  // do not apply for iteration not cutting on propagation
+  if (est.maxSagitta() <0 ) {


You can probably squeeze cpu a bit more if you compute x1 and y1 above only if (est.maxSagitta() >= 0 )

the compiler will take care AND est.maxSagitta() <0 is NOT the main use case

For my own education: how can the compiler know that "if (est.maxSagitta() <0 )" it can avoid making all computations from L38 to L48?

L38 and L39 most probably will be invoked anyhow as they are not inlined and may have side effects: this is true.
the rest is fully inlined and the minimum one can expect from a compiler is to optimize conditional assignment....

in any case: est.maxSagitta() <0 is "unlikely".

perrotta · 2017-05-02T07:56:25Z

RecoLocalTracker/SubCollectionProducers/src/JetCoreClusterSplitter.cc

-                fabs(sqrt(1.3 * 1.3 + 1.9 * 1.9 * jetZOverRho * jetZOverRho));
-            if (expSizeY < 1) expSizeY = 1.;
-            float expSizeX = 1.5;
+                std::sqrt((1.3f*1.3f) + (1.9f*1.9f) * jetZOverRho*jetZOverRho);


std::sqrt(1.69f+ 3.61f* jetZOverRhojetZOverRho); \ std::sqrt((1.3f1.3f) + (1.9f1.9f) * jetZOverRhojetZOverRho);

do not understand the comment: as I sais this class needs anyhow a major cleanup

the compiler takes care of proper folding constants...

perrotta · 2017-05-02T08:05:34Z

@VinInn I'm fine with this PR now: some little regression still visible, as expected, but all macroscopic effects potentially affecting physics performance have disappeared with the latest tunes. As a result of that tuning, improvements in cpu and perf do not seem as relevant as before: anyhow, some little cpu reduction is still visible for a few modules.
I found a couple of places in the code where probably some little more cpu can be further eroded (see review above): do you think it may be worth doing it?

cmsbuild · 2017-05-02T08:44:16Z

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

VinInn · 2017-05-02T08:46:00Z

@cmsbuild, please test

cmsbuild · 2017-05-02T08:46:16Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19492/console Started: 2017/05/02 10:47

cmsbuild · 2017-05-02T10:12:52Z

+1
Tested at: 0f16ca1
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18408/19492/summary.html

cmsbuild · 2017-05-02T10:12:55Z

Comparison job queued.

perrotta · 2017-05-02T10:19:17Z

+1
Some little regression still visible, as expected, but all macroscopic effects potentially affecting physics performance have disappeared with the latest tunes. As a result of that tuning, improvements in cpu and perf do not seem as relevant as before: anyhow, some cpu reduction is still visible for a few modules, as shown in #18408 (comment)

cmsbuild · 2017-05-02T10:19:38Z

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. @davidlange6, @smuzaffar

cmsbuild · 2017-05-02T13:12:55Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 3853 differences found in the comparisons
DQMHistoTests: Total files compared: 23
DQMHistoTests: Total histograms compared: 1778687
DQMHistoTests: Total failures: 21071
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 1757443
DQMHistoTests: Total skipped: 173
DQMHistoTests: Total Missing objects: 0
Checked 94 log files, 14 edm output root files, 23 DQM output files

davidlange6 · 2017-05-02T15:12:03Z

+1

VinInn added 2 commits April 19, 2017 16:03

introduce compatibility window

cc1b7cd

veirfy y as well

4aec9c5

cmsbuild added this to the Next CMSSW_9_1_X milestone Apr 19, 2017

cmsbuild added comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 19, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 19, 2017

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Apr 19, 2017

cmsbuild added comparison-pending tests-pending and removed comparison-notrun tests-rejected labels Apr 19, 2017

cmsbuild added tests-started and removed tests-pending labels Apr 19, 2017

cmsbuild added the comparison-available label May 1, 2017

perrotta reviewed May 2, 2017

View reviewed changes

optimize conditional assignment

0f16ca1

cmsbuild added comparison-pending tests-pending and removed comparison-available tests-approved labels May 2, 2017

cmsbuild added tests-started and removed tests-pending labels May 2, 2017

cmsbuild added tests-approved and removed tests-started labels May 2, 2017

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels May 2, 2017

cmsbuild added comparison-available and removed comparison-pending labels May 2, 2017

cmsbuild added orp-approved and removed orp-pending labels May 2, 2017

cmsbuild merged commit 9b7f92a into cms-sw:master May 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Speed up pattern recognition in Pixels by implementing a 5 sigma compatibility window in TkPixelMeasurementDet #18408

Speed up pattern recognition in Pixels by implementing a 5 sigma compatibility window in TkPixelMeasurementDet #18408

VinInn commented Apr 19, 2017 •

edited

cmsbuild commented Apr 19, 2017 •

edited by smuzaffar

VinInn commented Apr 19, 2017

VinInn commented Apr 19, 2017

VinInn commented Apr 19, 2017

cmsbuild commented Apr 19, 2017 •

edited

cmsbuild commented Apr 19, 2017

cmsbuild commented Apr 19, 2017

slava77 commented Apr 19, 2017

cmsbuild commented Apr 19, 2017 •

edited

cmsbuild commented Apr 19, 2017

perrotta May 2, 2017

VinInn May 2, 2017

perrotta May 2, 2017

VinInn May 2, 2017

perrotta May 2, 2017

VinInn May 2, 2017

VinInn May 2, 2017

perrotta May 2, 2017

perrotta commented May 2, 2017

cmsbuild commented May 2, 2017

VinInn commented May 2, 2017

cmsbuild commented May 2, 2017 •

edited

cmsbuild commented May 2, 2017

cmsbuild commented May 2, 2017

perrotta commented May 2, 2017

cmsbuild commented May 2, 2017

cmsbuild commented May 2, 2017

davidlange6 commented May 2, 2017

Speed up pattern recognition in Pixels by implementing a 5 sigma compatibility window in TkPixelMeasurementDet #18408

Speed up pattern recognition in Pixels by implementing a 5 sigma compatibility window in TkPixelMeasurementDet #18408

Conversation

VinInn commented Apr 19, 2017 • edited

cmsbuild commented Apr 19, 2017 • edited by smuzaffar

VinInn commented Apr 19, 2017

VinInn commented Apr 19, 2017

VinInn commented Apr 19, 2017

cmsbuild commented Apr 19, 2017 • edited

cmsbuild commented Apr 19, 2017

cmsbuild commented Apr 19, 2017

slava77 commented Apr 19, 2017

cmsbuild commented Apr 19, 2017 • edited

cmsbuild commented Apr 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

perrotta commented May 2, 2017

cmsbuild commented May 2, 2017

VinInn commented May 2, 2017

cmsbuild commented May 2, 2017 • edited

cmsbuild commented May 2, 2017

cmsbuild commented May 2, 2017

perrotta commented May 2, 2017

cmsbuild commented May 2, 2017

cmsbuild commented May 2, 2017

davidlange6 commented May 2, 2017

VinInn commented Apr 19, 2017 •

edited

cmsbuild commented Apr 19, 2017 •

edited by smuzaffar

cmsbuild commented Apr 19, 2017 •

edited

cmsbuild commented Apr 19, 2017 •

edited

cmsbuild commented May 2, 2017 •

edited