Remove redundant looping of elements for link tests in PFBlockAlgo #29054

hatakeyamak · 2020-02-27T15:28:41Z

PR description:

Tuning the looping logic so that we can remove the redundant looping of elements for link tests in PFBlockAlgo. This simple change should make the PFBlockAlgo [one of the most timing consuming part of PF] faster by a factor up to 1.8 with no change in outputs.

Using HLT's OnLine_HLT_GRun.py on:
/eos/cms/store/data/Run2018D/EphemeralHLTPhysics5/RAW/v1/000/324/897/00000/F5F13777-D664-C747-88AA-0459C1E35254.root
(1000 events)
FastReport

[Before]
CPU time avg.      when run  Real time avg.      when run     Alloc. avg.      when run   Dealloc. avg.      when run  Modules
        44.0 ms        44.0 ms        44.7 ms        44.7 ms       +3349 kB       +3349 kB       -2607 kB       -2607 kB    hltParticleFlowBlock
         0.9 ms        44.0 ms         0.9 ms        44.4 ms         +72 kB       +3434 kB         -55 kB       -2663 kB    hltParticleFlowBlockForTaus
         1.5 ms        43.0 ms         1.6 ms        45.2 ms        +112 kB       +3121 kB         -85 kB       -2381 kB    hltParticleFlowBlockReg

[With this PR]
        24.7 ms        24.7 ms        26.3 ms        26.3 ms       +3349 kB       +3349 kB       -2607 kB       -2607 kB    hltParticleFlowBlock
         0.5 ms        25.2 ms         0.6 ms        28.7 ms         +72 kB       +3438 kB         -56 kB       -2668 kB    hltParticleFlowBlockForTaus
         0.9 ms        24.5 ms         1.0 ms        27.8 ms        +112 kB       +3120 kB         -85 kB       -2382 kB    hltParticleFlowBlockReg

And, just for reference, if we also drop PS from PFBlockProducer for HLT as discussed in the TSG meeting, March 4th, (https://github.com/hatakeyamak/cmssw/pull/new/PFBlockProducerDropPS2)

        14.7 ms        14.7 ms        15.9 ms        15.9 ms       +1742 kB       +1742 kB       -1400 kB       -1400 kB    hltParticleFlowBlock
         0.3 ms        16.2 ms         0.4 ms        17.3 ms         +36 kB       +1752 kB         -29 kB       -1399 kB    hltParticleFlowBlockForTaus
         0.5 ms        12.6 ms         0.5 ms        12.9 ms         +52 kB       +1460 kB         -41 kB       -1146 kB    hltParticleFlowBlockReg

PR validation:

Check with 100 events from 2018 data that this change yields the identical list of PF candidates.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This is not backport.

@bendavid @jsalfeld @jpata

cmsbuild · 2020-02-27T15:33:35Z

The code-checks are being triggered in jenkins.

slava77 · 2020-02-27T23:00:53Z

code-check

if the bot lost the last attempt

slava77 · 2020-02-27T23:05:13Z

code-checks

cmsbuild · 2020-02-27T23:05:37Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-02-27T23:10:27Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29054/13937

This PR adds an extra 16KB to repository

cmsbuild · 2020-02-27T23:10:53Z

A new Pull Request was created by @hatakeyamak (Kenichi Hatakeyama) for master.

It involves the following packages:

RecoParticleFlow/PFProducer

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

cms-bot commands are listed here

slava77 · 2020-02-27T23:17:51Z

@cmsbuild please test

cmsbuild · 2020-02-27T23:18:09Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4925/console Started: 2020/02/28 00:24

cmsbuild · 2020-02-28T00:35:31Z

+1
Tested at: 8119750
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-39a3bd/4925/summary.html
CMSSW: CMSSW_11_1_X_2020-02-27-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-02-28T00:35:34Z

Comparison job queued.

cmsbuild · 2020-02-28T01:59:10Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2679706
DQMHistoTests: Total failures: 66
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2679321
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

cmsbuild · 2020-03-13T16:37:04Z

The code-checks are being triggered in jenkins.

cmsbuild · 2020-03-13T16:43:22Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29054/14187

This PR adds an extra 28KB to repository

cmsbuild · 2020-03-13T16:43:49Z

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

perrotta · 2020-03-13T16:47:54Z

please test

cmsbuild · 2020-03-13T16:48:12Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/5170/console Started: 2020/03/13 17:50

hatakeyamak · 2020-03-13T17:06:32Z

Thank you @perrotta for your review and also for initiating the test.

cmsbuild · 2020-03-13T18:02:12Z

+1
Tested at: f239cfb
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-39a3bd/5170/summary.html
CMSSW: CMSSW_11_1_X_2020-03-13-1100
SCRAM_ARCH: slc7_amd64_gcc820

cmsbuild · 2020-03-13T18:02:15Z

Comparison job queued.

cmsbuild · 2020-03-13T19:47:09Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 5 differences found in the comparisons
DQMHistoTests: Total files compared: 34
DQMHistoTests: Total histograms compared: 2680593
DQMHistoTests: Total failures: 66
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2680208
DQMHistoTests: Total skipped: 319
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 33 files compared)
Checked 147 log files, 16 edm output root files, 34 DQM output files

perrotta · 2020-03-16T10:17:54Z

+1

Redundant looping in PFBlockAlgo is removed, as explained in the PR description
Reco outputs are expected to remain the same: tiny differences observed in the jenkins tests are compatible with small numerical differences due to a different ordering of the PFBlocks, see also Remove redundant looping of elements for link tests in PFBlockAlgo #29054 (comment)

cmsbuild · 2020-03-16T10:18:16Z

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, @silviodonato, @fabiocos (and backports should be raised in the release meeting by the corresponding L2)

silviodonato · 2020-03-16T21:18:31Z

+1

Remove the redundant looping of elements for link tests

8119750

cmsbuild added this to the CMSSW_11_1_X milestone Feb 27, 2020

cmsbuild added code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Feb 27, 2020

hatakeyamak closed this Feb 27, 2020

hatakeyamak reopened this Feb 27, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Feb 27, 2020

cmsbuild added tests-started and removed tests-pending labels Feb 27, 2020

cmsbuild added tests-approved and removed tests-started labels Feb 28, 2020

hatakeyamak changed the title ~~Remove the redundant looping of elements for link tests in PFBlockAlgo~~ Remove redundant looping of elements for link tests in PFBlockAlgo Feb 28, 2020

cmsbuild added comparison-available and removed comparison-pending labels Feb 28, 2020

cmsbuild added comparison-pending tests-pending labels Mar 13, 2020

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 13, 2020

cmsbuild added tests-started and removed tests-pending labels Mar 13, 2020

cmsbuild added tests-approved and removed tests-started labels Mar 13, 2020

cmsbuild added comparison-available and removed comparison-pending labels Mar 13, 2020

cmsbuild removed the pending-signatures label Mar 16, 2020

cmsbuild added fully-signed reconstruction-approved and removed reconstruction-pending labels Mar 16, 2020

cmsbuild added orp-approved and removed orp-pending labels Mar 16, 2020

cmsbuild merged commit fad919f into cms-sw:master Mar 16, 2020

cmsbuild mentioned this pull request Mar 17, 2020

Alca packages: cleanup use of GCC11Compatibility.h #29121

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove redundant looping of elements for link tests in PFBlockAlgo #29054

Remove redundant looping of elements for link tests in PFBlockAlgo #29054

hatakeyamak commented Feb 27, 2020 •

edited

cmsbuild commented Feb 27, 2020

slava77 commented Feb 27, 2020

slava77 commented Feb 27, 2020

cmsbuild commented Feb 27, 2020

cmsbuild commented Feb 27, 2020

cmsbuild commented Feb 27, 2020

slava77 commented Feb 27, 2020

cmsbuild commented Feb 27, 2020 •

edited

cmsbuild commented Feb 28, 2020

cmsbuild commented Feb 28, 2020

cmsbuild commented Feb 28, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

perrotta commented Mar 13, 2020

cmsbuild commented Mar 13, 2020 •

edited

hatakeyamak commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

perrotta commented Mar 16, 2020 •

edited by silviodonato

cmsbuild commented Mar 16, 2020

silviodonato commented Mar 16, 2020

Remove redundant looping of elements for link tests in PFBlockAlgo #29054

Remove redundant looping of elements for link tests in PFBlockAlgo #29054

Conversation

hatakeyamak commented Feb 27, 2020 • edited

PR description:

PR validation:

if this PR is a backport please specify the original PR and why you need to backport that PR:

cmsbuild commented Feb 27, 2020

slava77 commented Feb 27, 2020

slava77 commented Feb 27, 2020

cmsbuild commented Feb 27, 2020

cmsbuild commented Feb 27, 2020

cmsbuild commented Feb 27, 2020

slava77 commented Feb 27, 2020

cmsbuild commented Feb 27, 2020 • edited

cmsbuild commented Feb 28, 2020

cmsbuild commented Feb 28, 2020

cmsbuild commented Feb 28, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

perrotta commented Mar 13, 2020

cmsbuild commented Mar 13, 2020 • edited

hatakeyamak commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

cmsbuild commented Mar 13, 2020

perrotta commented Mar 16, 2020 • edited by silviodonato

cmsbuild commented Mar 16, 2020

silviodonato commented Mar 16, 2020

hatakeyamak commented Feb 27, 2020 •

edited

cmsbuild commented Feb 27, 2020 •

edited

cmsbuild commented Mar 13, 2020 •

edited

perrotta commented Mar 16, 2020 •

edited by silviodonato