Does not use PF cluster cleaning in ECAL clusters.

[rafaellopesdesa]

Does not apply PF Cluster cleaning while making ECAL PF Clusters.

Please, refer to discussion in #17859

ECAL already provides cleaned rechits (by flagging them kOutOfTime, kWeird, kDiWeird). No additional cleaning is needed.

Attention: @emanueledimarco @Sam-Harper @knash @jainshilpi

[cmsbuild]

A new Pull Request was created by @rafaellopesdesa (Rafael Lopes de Sa) for master.

It involves the following packages:

RecoParticleFlow/PFClusterProducer

@perrotta, @cmsbuild, @slava77, @davidlange6 can you please review it and eventually sign? Thanks.
@mmarionncern, @lgray, @bachtis, @cbernet this is something you requested to watch as well.
@Muzaffar, @davidlange6, @smuzaffar you are the release manager for this.

cms-bot commands are listed here #13028

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/19327/console Started: 2017/04/21 20:27

[Sam-Harper]

thanks Rafael!

[cmsbuild]

+1
Tested at: ebaa739
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-18438/19327/summary.html

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

Comparison Summary:

• You potentially added 31 lines to the logs
• Reco comparison results: 1660 differences found in the comparisons
• DQMHistoTests: Total files compared: 23
• DQMHistoTests: Total histograms compared: 1826205
• DQMHistoTests: Total failures: 38959
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 1787073
• DQMHistoTests: Total skipped: 173
• DQMHistoTests: Total Missing objects: 0
• Checked 94 log files, 14 edm output root files, 23 DQM output files

[perrotta]

At the last reco meeting it was said that non applying the ECAL cluster cleaning at the PF level (as in this PR) should have been completely transparent, because of the cleaning already applied at the recHits level. Indeed, the effect is extremely small, although not being completely null. Below there are a couple of examples from the 10025.0 wf (ZEE_13TeV)

(particleFlowSuperClusterECAL_particleFlowBasicClusterECALBarrel_RECO_obj_correctedEnergy)

(Photons_gedPhotons__RECO_obj_energy)

I couldn't find any visible regression in the few real data workflows I tested, instead: quite likely some larger stat is needed.

For me this is perfectly acceptable, but please @rafaellopesdesa @emanueledimarco @Sam-Harper @knash @jainshilpi confirm that this is also fine for you. Once this pull request is merged (and the corresponding one for HLT), then #17859 will act only on HF, as wanted, and it can be also accepted.

[Sam-Harper]

Hmm, to me this is a bug fix so any change is bringing it towards the correct behaviour. The real test is on data though as its spike cleaning. Again ECAL is already doing the spike cleaning so there shouldnt be an issue unless there is actually a problem with the ECAL spike cleaning (unlikely...)

[perrotta]

+1
According to #18438 (comment) and #18438 (comment) (who confirmed that the small regression seen is not by itself a sympton of some non expected behavior)

[cmsbuild]

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

[davidlange6]

+1

[knash]

@perrotta One point here is that in the ECAL spike cleaning case there is currently the "double spike cleaning" which as far as I can tell has been working. So in that case, completely turning off spike cleaning represents a change.

[Sam-Harper]

@knash just to reassure you, the double spike cleaning is already done at when making the PFRecHits via kDiWeird, see : https://github.com/cms-sw/cmssw/blob/master/RecoParticleFlow/PFClusterProducer/interface/PFRecHitQTests.h#L498-L500

Again this class should not exist without some justification because as implemented it tries to do exactly the same thing twice (and as you have found, does it poorly).

[knash]

@Sam-Harper Hello, good to know.

I just wanted to tie up the potential last confusion here as my original comment was that the spike cleaning bug caused the spike cleaning to be essentially turned off. If that was the case then the differences observed in the photon energy shown earlier would not make any sense.

However it is just the single spike cleaning that is turned off by the bug, so by turning of the cleaning completely you can still observe a difference due to the double spikes.