minimal fix to use rep, not position.eta/phi

[VinInn]

very minimal change.
major speedup
NO regression.
8_1_X version produces regression due to the consistent use of double

[cmsbuild]

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

It involves the following packages:

RecoParticleFlow/PFClusterProducer

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

[VinInn]

@cmsbuild, please test

[VinInn]

@fwyzard , this is for HLT

[cmsbuild]

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

[cmsbuild]

+1
Tested at: 3d23244
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14057/12359/summary.html

[cmsbuild]

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

[slava77]

it may be better to put this to 81X as well first; so that auto-forward-port doesn't bring surprises depending on the time order of #14058 merging (it will either make 14058 not mergeable or may even modify something out of order if 14058 is merged first .. it's unclear if the context lines are fully overlapping or not)

[VinInn]

This is NOT supposed to be autoforwarded.
It will cause merging issue with #14058.
You can leave it pending while reviewing #14058.
It just demonstrates the silly origin of the regression observed there.

[slava77]

@VinInn
forward port is automated
we can't stop it unless we generate an anti-commit or have the same commit in 81X

[VinInn]

@slava77 , ok keep this as an alternative to #14058.
If CMS decides that #14058 (and future PR about slimming PFRecHit) are not suited for backport
we integrate this and solve merge issues in 81X,
if instead we go on with full backport we will close this one.

[davidlt]

We can block back port by merging manually with -s ours into CMSSW_8_1_X. That basically will import the history, but no changes. So basically doing so trashes the history.

[cvuosalo]

+1

For #14057 3d23244

Speeding up PFCluster computations. There should be no change in monitored quantities.

This PR has some overlap with the already merged #14058 for 81X.

The code changes are satisfactory, and Jenkins tests against baseline CMSSW_8_0_X_2016-04-13-1100 show no significant differences, as expected. An extended test of workflow 25202.0_TTbar_13 with 70 events against baseline CMSSW_8_0_X_2016-04-12-2300 also shows no significant differences. Overall CPU time and memory use do not change significantly, but there is timing improvement in the affected modules (times exclude first event):

  delta/mean delta/orJob     original                   new       module name
  ---------- ------------     --------                  ----       ------------
   -1.518303      -0.01%         9.33 ms/ev ->         1.28 ms/ev hltParticleFlowClusterHCALForEgamma
   -1.517276      -0.01%         9.25 ms/ev ->         1.27 ms/ev hltParticleFlowClusterHCALForMuons
   -1.513000      -0.01%         9.27 ms/ev ->         1.29 ms/ev hltParticleFlowClusterHCAL
   -1.499091      -0.01%         7.45 ms/ev ->         1.07 ms/ev hltParticleFlowClusterHCALForTkMuons
   -1.496918      -0.04%         7.44 ms/ev ->         1.07 ms/ev particleFlowClusterHCAL
   -0.912711      -0.06%        15.13 ms/ev ->         5.65 ms/ev particleFlow
  ---------- ------------     --------                  ----       ------------

Another time measure also shows improvement:

    particleFlowClusterHCAL      4.81418 ms/ev -> 0.756229 ms/ev

[cmsbuild]

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, @Degano, @smuzaffar

[VinInn]

I suggest not to merge this in 8_0_X for the time being. At some point we need to understand if and when to backport all HLT speed ups currently in 8_1_X taking into account that some small regression will anyhow be present in reco as well

[VinInn]

This is not supposed to be forward ported!

[slava77]

I think that there are enough context lines to not let this go through to 81X
... the bot haven't done any 80X=>81X merges in a while now.
@smuzaffar please clarify if something is in the way

[davidlt]

@slava77 there is a merge conflict (CalibTracker/SiStripDCS) since tomorrow's late evening, which needs to resolved.

[smuzaffar]

@slava77 and @davidlt , conflicts are now resolved. changes from this PR are not forward ported