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

SpeedUp ParticleFlow for HLT #13959

Merged
merged 24 commits into from Apr 13, 2016
Merged

SpeedUp ParticleFlow for HLT #13959

merged 24 commits into from Apr 13, 2016

Conversation

VinInn
Copy link
Contributor

@VinInn VinInn commented Apr 6, 2016

Technical mods to ParticleFlow (and some core classes) to speedup creation for PFRecHIt and PFClusters.
Most of it is trivial.
I did not observe any regression on TTBAR PU35 reco.
Includes contribution by @lgray

added major speedup in ES geometry and EcalRecHit creation

@lgray , @fwyzard

p.s. shrink_to_fit is still calling RefCore copy constructor instead of moving as it should....

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

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

It involves the following packages:

CommonTools/Utils
DataFormats/Common
DataFormats/ParticleFlowReco
Geometry/EcalAlgo
RecoParticleFlow/PFClusterProducer

@smuzaffar, @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mmarionncern, @wmtan, @makortel, @rafaellopesdesa, @wddgit, @lgray, @argiro, @bachtis this is something you requested to watch as well.
@slava77, @Degano, @smuzaffar you are the release manager for this.

cms-bot commands are list here #13028

@lgray
Copy link
Contributor

lgray commented Apr 6, 2016

@cmsbuild please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 6, 2016

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

@Dr15Jones
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

Pull request #13959 was updated. @smuzaffar, @civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @slava77, @davidlange6 can you please check and sign again.

@VinInn
Copy link
Contributor Author

VinInn commented Apr 12, 2016

Let me stress that I am not here to put PF code in line with CMSSW standards.
Please try to find and appoint proper person-power to maintain (and optimize) such a critical component of CMS data processing and physics analysis

@cmsbuild
Copy link
Contributor

-1

Tested at: 5d87998
I found errors in the following unit tests:

---> test runtestTqafTopEventSelection had ERRORS

you can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13959/12300/summary.html
'List

@VinInn
Copy link
Contributor Author

VinInn commented Apr 12, 2016

can we get rid of this runtestTqafTopEventSelection ???

@cmsbuild
Copy link
Contributor

@VinInn
Copy link
Contributor Author

VinInn commented Apr 12, 2016

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

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

@cmsbuild
Copy link
Contributor

-1

Tested at: 99a4c72
I found errors in the following unit tests:

---> test runtestTqafTopEventSelection had ERRORS

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

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
ccc64b0
You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13959/12331/git-log-recent-commits
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13959/12331/git-merge-result

'List

@cmsbuild
Copy link
Contributor

@slava77
Copy link
Contributor

slava77 commented Apr 13, 2016

+1

for #13959 99a4c72

  • changes in the code are in line with the goal: to speedup PF without regressions
  • jenkins tests pass and comparisons with baseline show no differences
  • the last commits since 38c7739 should introduce no differences run time; see comments on higher stat tests in SpeedUp ParticleFlow for HLT #13959 (comment)

@VinInn
Copy link
Contributor Author

VinInn commented Apr 13, 2016

I am sorry with all those who have already signed once, one more signature of yours is requested.
Thanks.

@Dr15Jones
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