Bugfix: Ensure by force that the deepest root of the connected graph is always found in PFBA

[lgray]

Quick Union algorithm was not always returning the deepest root available in a connected graph.
This is a deficiency in the underlying algorithm, luckily it is easy to patch out (just keep looking for roots of roots).
I'll fix the underlying cause in the future, thankfully it's iterlog-N on average.

I also added in a clause to skip direct connection tests if two nodes are already connected. Should help with speed. If there is incidental slow down compared to yesterday's IB I will remove it.

[lgray]

@cmsbuild please test

[cmsbuild]

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

[cmsbuild]

A new Pull Request was created by @lgray (Lindsey Gray) for CMSSW_8_1_X.

It involves the following packages:

RecoParticleFlow/PFProducer

@cmsbuild, @cvuosalo, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@mmarionncern, @rafaellopesdesa, @bachtis, @cbernet 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

[cmsbuild]

+1
Tested at: cde485d
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14844/13457/summary.html

[cmsbuild]

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

@slava77 comparisons for the following workflows were not done due to missing matrix map:

• 134.911_RunSinglePh2015D+RunSinglePh2015D+HLTDR2_25ns+RECODR2_25nsreHLT+HARVESTDR2

[slava77]

Looking at 25202 (ttbar PU35)
There is about 20% increase in PFBlock producers in HLT

  delta/mean delta/orJob     original                   new       module name
  ---------- ------------     --------                  ----       ------------
   +0.198985      +0.04%       119.77 ms/ev ->       146.24 ms/ev hltParticleFlowBlockForTaus
   +0.189205      +0.04%       125.99 ms/ev ->       152.31 ms/ev hltParticleFlowBlock
   +0.185571      +0.03%       123.28 ms/ev ->       148.50 ms/ev hltParticleFlowBlockReg

comparing with full revert of the changes in PFBA to 810pre3 version

   -0.800458      -0.24%       341.41 ms/ev ->       146.24 ms/ev hltParticleFlowBlockForTaus
   -0.791172      -0.24%       342.88 ms/ev ->       148.50 ms/ev hltParticleFlowBlockReg
   -0.773167      -0.23%       344.30 ms/ev ->       152.31 ms/ev hltParticleFlowBlock

there is still more than a factor of two improvement

and about 12% increase in RECO from this PR:

   +0.122525      +0.03%       171.52 ms/ev ->       193.91 ms/ev particleFlowBlock

comparing with full revert of the changes in PFBA to 810pre3 version

   -0.930843      -0.03%        33.72 ms/ev ->        12.30 ms/ev pfNoPileUp
   -0.929365      -0.03%        33.68 ms/ev ->        12.31 ms/ev pfNoPileUpIso
   -0.924957      -0.03%        39.68 ms/ev ->        14.58 ms/ev pfNoPileUpIsoPFBRECO
   -0.917386      -0.03%        39.64 ms/ev ->        14.71 ms/ev pfNoPileUpEI
   -0.541508      -0.02%        38.69 ms/ev ->        22.20 ms/ev pfNoPileUpJME
   -0.539415      -0.02%        40.23 ms/ev ->        23.14 ms/ev pfNoPileUpJMEEI
   -0.161522      -0.04%       227.98 ms/ev ->       193.91 ms/ev particleFlowBlock

the net effect from 14138 to 14441 to 14754 to 14844 for particleFlowBlock appears to be close to 20%, combined with gains in pfNoPileUp*, this is still pretty good.

[slava77]

+1

for #14844 cde485d

• changes are in line with the description
• jenkins tests pass and comparisons with baseline show no difference
• local tests in CMSSW_8_1_X_2016-06-09-1100 show that crashes observed in the baseline are gone
  • the baseline had crashes in 1313.0 (on the 309th event), 1338.0 (on the 519th event), and the one reported in https://hypernews.cern.ch/HyperNews/CMS/get/recoDevelopment/1448/1/2.html.
  • The first two were actually present in local tests of Bugfix: Run forward and backwards when looking for links in PFBA #14754 but I didn't notice them. So, Bugfix: Run forward and backwards when looking for links in PFBA #14754 wasn't supposed to be signed.

[cmsbuild]

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

[slava77]

@davidlange6
please include this in pre7

[lgray]

@slava77 Thanks. I'll go through the aftermath here with perf stat and see if we can't get anything back.

[davidlange6]

+1