Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport of #14179: Speed up Jet Producer for HLT #15226

Merged
merged 6 commits into from Jul 26, 2016

Conversation

makortel
Copy link
Contributor

This PR backports #14179 from CMSSW_8_1_X to CMSSW_8_0_X. Original description by @VinInn:

factor 2 speed up mostly by inlining area computation.
Solid 0.6% total speedup of HLT at PU22.
The JetProducer is in the need of more cleanup
(and tower revertexing is still utterly slow)
A more serious speedup could be obtained applying a cutoff to the jet energy...

No regression expected offline as the code in question runs only at HLT...

Tested in CMSSW_8_0_14, no changes expected in RECO quantities in there either.

@fwyzard @gennai @VinInn @slava77

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @makortel (Matti Kortelainen) for CMSSW_8_0_X.

It involves the following packages:

RecoJets/JetProducers

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@TaiSakuma, @jdolen, @ahinzmann, @rappoccio, @yslai, @nhanvtran, @schoef, @mariadalfonso this is something you requested to watch as well.
@slava77, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@slava77
Copy link
Contributor

slava77 commented Jul 19, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 19, 2016

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/14173/console

@cmsbuild
Copy link
Contributor

@cmsbuild
Copy link
Contributor

@cvuosalo
Copy link
Contributor

Timing tests are in progress...

@fwyzard
Copy link
Contributor

fwyzard commented Jul 20, 2016

tracked at #15151

@cvuosalo
Copy link
Contributor

HLT/RECO timing tests in progress...

@cvuosalo
Copy link
Contributor

An HLT timing test was performed using a config provided by TSG and run on file /store/group/dpg_trigger/comm_trigger/TriggerStudiesGroup/Timing/run276935/HLTPhysics_Run276935_LS80-220.root. The PR was compared to baseline CMSSW_8_0_14.

CPU time shrank by about 1%. The Jet Producer showed improvement:

  delta/mean delta/orJob     original                   new       module name
  ---------- ------------     --------                  ----       ------------
   -0.526124      -0.16%         7.77 ms/ev ->         4.53 ms/ev hltAK4PFJetsForTaus
   -0.520573      -0.16%         7.96 ms/ev ->         4.67 ms/ev hltAK4PFJets
   -0.377638      -0.07%         4.37 ms/ev ->         2.98 ms/ev hltAK4CaloJets
  ---------- ------------     --------                  ----       ------------
Job total:  2.02722 s/ev ==> 2.00935 s/ev

Another measure of the mean time to process an event also shows a 1% improvement:

HLT_Validation_Plot.pdf

@slava77
Copy link
Contributor

slava77 commented Jul 22, 2016

On 7/22/16 3:29 PM, Carl Vuosalo wrote:

An HLT timing test was performed using a config provided by TSG and run
on file
/store/group/dpg_trigger/comm_trigger/TriggerStudiesGroup/Timing/run276935/HLTPhysics_Run276935_LS80-220.root.
The PR was compared to baseline CMSSW_8_0_14.

CPU time shrank by about 1%. The Jet Producer showed improvement:

|delta/mean delta/orJob original new module name



-0.526124 -0.16% 7.77 ms/ev -> 4.53 ms/ev hltAK4PFJetsForTaus
-0.520573 -0.16% 7.96 ms/ev -> 4.67 ms/ev hltAK4PFJets
-0.377638 -0.07% 4.37 ms/ev -> 2.98 ms/ev hltAK4CaloJets


Job total: 2.02722
s/ev ==> 2.00935 s/ev |

This looks like a count per running module.
Could you please get the numbers per event.
(jet clustering doesn't have any massive initialization. So, just the
job report totals would suffice).

Thank you.

Another measure of the mean time to process an event also shows a 1%
improvement:

HLT_Validation_Plot.pdf
https://github.com/cms-sw/cmssw/files/379170/HLT_Validation_Plot.pdf


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#15226 (comment), or
mute the thread
https://github.com/notifications/unsubscribe-auth/AEdcbkb6UiefpmrOx6yGc72mw7oUKe0Rks5qYUQvgaJpZM4JPj1-.

@cvuosalo
Copy link
Contributor

+1

For #15226 c7a2488

Speeding up the jet producer for HLT. There should be no change in monitored quantities.

#14179 is the 81X version of this PR , and it was included 810pre4.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_0_X_2016-07-17-2300 show no significant differences, as expected. A test of workflow 25202.0_TTbar_13 with 70 events against baseline CMSSW_8_0_14 also shows no significant differences. An HLT timing test described above shows about a 1% improvement in timing. Corrected per-module timing measurements (compared to those shown above) also show an overall 1% improvement:

  delta/mean delta/orJob     original                   new       module name
  ---------- ------------     --------                  ----       ------------
   -0.520574      -0.21%         0.85 ms/ev ->         0.50 ms/ev hltAK4PFJets
   -0.377634      -0.23%         1.19 ms/ev ->         0.81 ms/ev hltAK4CaloJets
  ---------- ------------     --------                  ----       ------------
Job total:  0.162549 s/ev ==> 0.161243 s/ev

@cmsbuild
Copy link
Contributor

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

@slava77
Copy link
Contributor

slava77 commented Jul 25, 2016

@cvuosalo
how do we get an overall 1% improvement from the two modules time change?
They show 0.35 +0.38 ms/ev gain, compared to the total time of 0.16 s/ev, it is just under 0.5%

@cvuosalo
Copy link
Contributor

@slava77: Yes, the total of the per-module times show a 0.4% improvement in timing. The overall times (including in this plot https://github.com/cms-sw/cmssw/files/379170/HLT_Validation_Plot.pdf) show about a 1% improvement, but the bulk of that might just be noise.

@davidlange6
Copy link
Contributor

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants