HBHE cleanup and speedup #27726

mariadalfonso · 2019-08-08T12:29:58Z

This PR save 10% of the CPU speed in view of the Run3.
Changes are expected within numerical precision level in the E/ArrivalTime of recHits.
Pass clang formats checks.

Validation plots are here
PR27726.pdf

Main gain is in the updatePulseShape and updateCov functions of Mahi.
Below igprof report (test done on Run2018A_HLTPhysics run#316457 PU50 ).

ORIGINAL:

           13.4  .........      62.43 / 63.03        MahiFit::phase1Apply(HBHEChannelInfo const&, float&, float&, bool&, float&) const [45]
[46]       13.4      62.43       3.03 / 59.40      MahiFit::doFit(std::array<float, 3ul>&, int) const
            9.7  .........      45.45 / 45.45        MahiFit::minimize() const [47]
            1.7  .........       7.75 / 7.75         MahiFit::updatePulseShape(double, Eigen::Matrix<double, -1, 1, 0, 19, 1>&, Eigen::Matrix<double, -1, 1, 0, 19, 1>&, Eigen::Matrix<$
            1.3  .........       6.19 / 6.19         MahiFit::calculateArrivalTime() const [80]
            0.0  .........       0.00 / 0.89         _init [204]
            0.0  .........       0.00 / 0.12         _dl_runtime_resolve_xsave [757]

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
            9.7  .........      45.45 / 62.43        MahiFit::doFit(std::array<float, 3ul>&, int) const [46]
[47]        9.7      45.45       0.52 / 44.92      MahiFit::minimize() const
            4.2  .........      19.42 / 19.42        MahiFit::nnls() const [53]
            3.5  .........      16.26 / 16.26        MahiFit::updateCov() const [55]
            1.2  .........       5.54 / 5.54         MahiFit::onePulseMinimize() const [83]
            0.8  .........       3.63 / 3.63         MahiFit::calculateChiSq() const [94]
            0.0  .........       0.07 / 0.89         _init [204]

NEW

           11.0  .........      57.34 / 57.54        MahiFit::phase1Apply(HBHEChannelInfo const&, float&, float&, bool&, float&) const [45]
[46]       11.0      57.34       3.78 / 53.55      MahiFit::doFit(std::array<float, 3ul>&, int) const
            8.1  .........      42.62 / 42.62        MahiFit::minimize() const [47]
            1.1  .........       5.76 / 5.76         MahiFit::updatePulseShape(double, Eigen::Matrix<double, -1, 1, 0, 19, 1>&, Eigen::Matrix<double, -1, 1, 0, 19, 1>&, Eigen::Matrix<$
le, -1, -1, 0, 19, 19>&) const [78]
            1.0  .........       5.17 / 5.17         MahiFit::calculateArrivalTime() const [81]
            0.0  .........       0.00 / 0.79         _init [211]

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
            8.1  .........      42.62 / 57.34        MahiFit::doFit(std::array<float, 3ul>&, int) const [46]
[47]        8.1      42.62       0.32 / 42.29      MahiFit::minimize() const
            4.6  .........      24.09 / 24.09        MahiFit::nnls() const [48]
            2.1  .........      10.73 / 10.73        MahiFit::updateCov() const [60]
            0.9  .........       4.65 / 4.65         MahiFit::onePulseMinimize() const [85]
            0.5  .........       2.80 / 2.80         MahiFit::calculateChiSq() const [107]
            0.0  .........       0.02 / 0.79         _init [211]

more cleanup clang format

cmsbuild · 2019-08-08T12:30:30Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-08-08T12:38:37Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27726/11348

This PR adds an extra 32KB to repository

cmsbuild · 2019-08-08T12:39:03Z

A new Pull Request was created by @mariadalfonso for master.

It involves the following packages:

RecoLocalCalo/HcalRecAlgos

@perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@argiro this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

slava77 · 2019-08-08T13:11:23Z

@cmsbuild please test

cmsbuild · 2019-08-08T13:11:52Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/1906/console Started: 2019/08/08 15:12

cmsbuild · 2019-08-08T14:44:46Z

+1
Tested at: 19e5247
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-657e20/1906/summary.html

cmsbuild · 2019-08-08T14:44:50Z

Comparison job queued.

cmsbuild · 2019-08-08T16:53:04Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 66 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 2715989
DQMHistoTests: Total failures: 248
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2715412
DQMHistoTests: Total skipped: 329
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

mariadalfonso · 2019-08-14T10:52:19Z

@slava77
Let me know if something is needed from my side for this PRreview

@VinInn FYI

slava77 · 2019-08-14T23:08:20Z

Let me know if something is needed from my side for this PRreview

this is ~next on my list, I didn't look at the PR yet.

slava77 · 2019-08-22T13:27:03Z

@cmsbuild please test
just to refresh the two week old tests

cmsbuild · 2019-08-22T13:27:33Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2114/console Started: 2019/08/22 15:28

cmsbuild · 2019-08-27T13:45:08Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-08-27T13:52:48Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27726/11658

This PR adds an extra 32KB to repository

cmsbuild · 2019-08-27T13:53:16Z

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

slava77 · 2019-08-27T14:24:52Z

@cmsbuild please test

cmsbuild · 2019-08-27T14:25:19Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2222/console Started: 2019/08/27 16:26

cmsbuild · 2019-08-27T15:55:34Z

+1
Tested at: 76749bc
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-657e20/2222/summary.html

cmsbuild · 2019-08-27T15:55:38Z

Comparison job queued.

cmsbuild · 2019-08-27T17:33:35Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 68 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2939508
DQMHistoTests: Total failures: 250
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2938917
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 145 log files, 15 edm output root files, 34 DQM output files

slava77 · 2019-08-28T17:13:09Z

+1

for #27726 76749bc

code changes are in line with the PR description and the follow up review
jenkins tests pass and comparisons with the baseline show only small differences likely related to differences in HBHE reco at the numerical precision level
- manual check on wf 140.53 shows that the hit energy for 155406 is the same and differs in 24 hits by xE-7, consistent with possible float precision differences
I did not check the CPU time. The changes in the code are expected to reduce computations, in line with the profiler report provided with the PR description

cmsbuild · 2019-08-28T17:15:42Z

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 will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

kpedro88 · 2019-08-28T21:06:45Z

+1

abdoulline · 2019-09-06T08:29:11Z

Just to add (for the record, better late than never...) -
our usual single-pion test (for Run3 specifically) shows no any difference in 11_0_0_pre7 (with this PR included) vs 10_0_0_pre6 :
https://cms-cpt-software.web.cern.ch/cms-cpt-software/General/Validation/SVSuite/HCAL/calo_scan_single_pi/11_X/11_0_0_pre7_run3_vs_11_0_0_pre6_run3_SinglePi/

cleanup

19e5247

more cleanup clang format

cmsbuild added this to the CMSSW_11_0_X milestone Aug 8, 2019

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 8, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 8, 2019

cmsbuild added tests-started and removed tests-pending labels Aug 8, 2019

cmsbuild added tests-approved and removed tests-started labels Aug 8, 2019

cmsbuild added comparison-available and removed comparison-pending labels Aug 8, 2019

cmsbuild removed comparison-available tests-approved labels Aug 22, 2019

cmsbuild added code-checks-pending comparison-pending tests-pending labels Aug 27, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 27, 2019

cmsbuild added tests-started and removed tests-pending labels Aug 27, 2019

cmsbuild added tests-approved and removed tests-started labels Aug 27, 2019

cmsbuild added comparison-available and removed comparison-pending labels Aug 27, 2019

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Aug 28, 2019

cmsbuild added orp-approved and removed orp-pending labels Aug 28, 2019

cmsbuild merged commit 5de33bb into cms-sw:master Aug 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HBHE cleanup and speedup #27726

HBHE cleanup and speedup #27726

mariadalfonso commented Aug 8, 2019 •

edited

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

slava77 commented Aug 8, 2019

cmsbuild commented Aug 8, 2019 •

edited

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

mariadalfonso commented Aug 14, 2019

slava77 commented Aug 14, 2019

slava77 commented Aug 22, 2019

cmsbuild commented Aug 22, 2019 •

edited

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

slava77 commented Aug 27, 2019

cmsbuild commented Aug 27, 2019 •

edited

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

slava77 commented Aug 28, 2019

cmsbuild commented Aug 28, 2019

kpedro88 commented Aug 28, 2019

abdoulline commented Sep 6, 2019

HBHE cleanup and speedup #27726

HBHE cleanup and speedup #27726

Conversation

mariadalfonso commented Aug 8, 2019 • edited

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

slava77 commented Aug 8, 2019

cmsbuild commented Aug 8, 2019 • edited

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

cmsbuild commented Aug 8, 2019

mariadalfonso commented Aug 14, 2019

slava77 commented Aug 14, 2019

slava77 commented Aug 22, 2019

cmsbuild commented Aug 22, 2019 • edited

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

slava77 commented Aug 27, 2019

cmsbuild commented Aug 27, 2019 • edited

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

cmsbuild commented Aug 27, 2019

slava77 commented Aug 28, 2019

cmsbuild commented Aug 28, 2019

kpedro88 commented Aug 28, 2019

abdoulline commented Sep 6, 2019

mariadalfonso commented Aug 8, 2019 •

edited

cmsbuild commented Aug 8, 2019 •

edited

cmsbuild commented Aug 22, 2019 •

edited

cmsbuild commented Aug 27, 2019 •

edited