Slim down PFRecHit

[VinInn]

made on top of #14058 to avoid merge conflicts.

What has been done?
removed all positional content
add a pointer to the corresponding CaloCell (itself improved to satifly PF needs)
forward all positional access methods to caloCell
made neighebours a single vector (transient, so just index in the collection)
move to float for energy and time

Caveat:
This would have been all trivial if was not for the depth correctiion in EF.
CMSSW_8_1_X...VinInn:SlimPFHit81#diff-4a5cef800feb39968287c389658617bcL85
What I did is simply to double the geometry of EF: now each IdealZPrism owns a copy of itslelf with PF depth-correction applied (as magic numbers)
CMSSW_8_1_X...VinInn:SlimPFHit81#diff-1a95dcb4933ebfc4bbc6f2bf984cc933R47

what will not work?
anybody accessing positional content of PFRecHIt from reco-files w/o protection.
In the released code this affects only FWPFCandidateDetailView (note that instead FWPFCandidateWithHitsProxyBuilder works out of the box, so a solution internal to FireWorks exists
as acknowledged by Alja)
validate.C will require to drop few histos (eta/phi of PFRecHit)
as already noted no DQM/Validatation code exists for PF, so no problem there ;-)

what we gain?
a solid 6% of total time of HLT above what already gained so far.

minor regression observed most probably related to numerics
http://innocent.home.cern.ch/innocent/regress/pu35_SlimPFHit3/

[cmsbuild]

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

It involves the following packages:

DataFormats/Math
DataFormats/ParticleFlowReco
Fireworks/ParticleFlow
Geometry/CaloGeometry
Geometry/HcalEventSetup
Geometry/HcalTowerAlgo
RecoParticleFlow/PFClusterProducer
RecoParticleFlow/PFClusterTools
RecoParticleFlow/PFProducer

@civanch, @Dr15Jones, @cvuosalo, @ianna, @mdhildreth, @cmsbuild, @alja, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@ghellwig, @mmarionncern, @makortel, @rafaellopesdesa, @lgray, @alja, @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

[VinInn]

@cmsbuild , please test

[cmsbuild]

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

[VinInn]

@fwyzard as well

[VinInn]

"fixes" needed to validate.C

 diff  ~/code/cms-reco-tools/validate.C .
1642,1643c1642,1643
<       plotvar("recoPFRecHits_particleFlowRecHitHO_Cleaned_"+recoS+".obj.position().eta()");
<       plotvar("recoPFRecHits_particleFlowRecHitHO_Cleaned_"+recoS+".obj.position().phi()");
---
>       // plotvar("recoPFRecHits_particleFlowRecHitHO_Cleaned_"+recoS+".obj.position().eta()");
>       // plotvar("recoPFRecHits_particleFlowRecHitHO_Cleaned_"+recoS+".obj.position().phi()");
1648,1649c1648,1649
<       plotvar("recoPFRecHits_particleFlowRecHitECAL_Cleaned_"+recoS+".obj.position().eta()");
<       plotvar("recoPFRecHits_particleFlowRecHitECAL_Cleaned_"+recoS+".obj.position().phi()");
---
>       // plotvar("recoPFRecHits_particleFlowRecHitECAL_Cleaned_"+recoS+".obj.position().eta()");
>       // plotvar("recoPFRecHits_particleFlowRecHitECAL_Cleaned_"+recoS+".obj.position().phi()");
1654,1655c1654,1655
<       plotvar("recoPFRecHits_particleFlowRecHitHCAL_Cleaned_"+recoS+".obj.position().eta()");
<       plotvar("recoPFRecHits_particleFlowRecHitHCAL_Cleaned_"+recoS+".obj.position().phi()");
---
>       // plotvar("recoPFRecHits_particleFlowRecHitHCAL_Cleaned_"+recoS+".obj.position().eta()");
>       // plotvar("recoPFRecHits_particleFlowRecHitHCAL_Cleaned_"+recoS+".obj.position().phi()");
1660,1661c1660,1661
<       plotvar("recoPFRecHits_particleFlowRecHitPS_Cleaned_"+recoS+".obj.position().eta()");
<       plotvar("recoPFRecHits_particleFlowRecHitPS_Cleaned_"+recoS+".obj.position().phi()");
---
>       // plotvar("recoPFRecHits_particleFlowRecHitPS_Cleaned_"+recoS+".obj.position().eta()");
>       // plotvar("recoPFRecHits_particleFlowRecHitPS_Cleaned_"+recoS+".obj.position().phi()");

[alja]

+1

[cmsbuild]

+1
Tested at: 60b5314
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14151/12497/summary.html

[cmsbuild]

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

[slava77]

the release diverged pretty quickly.
Time to rebase.

[VinInn]

Wonder who touched these files....

[ianna]

please test

[VinInn]

should we really run tests once more?

[cmsbuild]

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

[ianna]

@VinInn - I'm sure by the time it is merged, the tests will be done

[VinInn]

@ianna, ok. please just merge again on your area and test the writer yourself (I did in my area)

[ianna]

+1

[Dr15Jones]

+1

[slava77]

+1

for #14151 fe93fd7

• changes in the code are in line with the description
• jenkins tests passed for ef463c1 and extended tests showed no issues either (see notes earlier)
  • validate.C script was updated for this validate.C updates: miniaod validation with fwlite (partial integration) and a few minor updates cms-bot#669
• jenkins tests for fe93fd7 have gone through compilation OK and tests are running; the code changes since ef463c1 are not relevant to jenkins tests triggered here
• @Dr15Jones assessment allows dependence on this Geometry subsystem part; Explicitly add allowed Geometry packages to FWLite #14253

[cmsbuild]

+1
Tested at: fe93fd7
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-14151/12643/summary.html

[cmsbuild]

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

[VinInn]

in reco is very difficult to assess timing differences in PFBlock: I observe differences even in functions not accessing PFRecHits
I do not exclude that we are paying a price of mixed math in some of the LinkByRecHit that I have not fixed (actually I fixed only testECALAndPSByRecHit)
I can surely improve all others along the same lines
As @LGrey suggested one my extend the use of KDtree to other linkers once we identify the more relevant for reco and HLT
In any case it seems that hltParticleFlowBlockForTaus runs for all TTBAR events while at HLT its contribution is actually much smaller, still something to to watch

[VinInn]

Indeed HLT for 252002 step2 is dominated by PFBlock (25%)
At least on my machine I do not see any slowdown caused this PR.