Adopt ChargedHadronPFTrackIsolationProducer for PFTICL candidates

[hatakeyamak]

PR description:

[address issues reported by HGCAL folks]
Update ChargedHadronPFTrackIsolationProducer so that we won’t get a crash even when --customise RecoHGCal/TICL/iterativeTICL_cff.injectTICLintoPF is specified. PF candidates from TICL don’t go through PFBlocks. This is generally fine, but when rawECALEnergy and rawHCALEnergy are set to TICL PF candidates for JetMET to perform PF hadron calibration, it leads to a logic error. More in detail, after setting these quantities [2.1], the ChargedHadronPFTrackIsolationProducer [2.2] crashes because it checks the PFElement of type TRACK to identify isolated charged hadrons. (It was not crashing before as the rawECALEnergy and rawECALEnergy are 0, effectively bypassing this check.)

This isolation check is relevant for PF candidates coming from PFAlgo, where multiple tracks can associate to the same calorimeter clusters, but this is not relevant for PF candidates from TICL. In this PR, we consider PF charged candidates from TICL isolated, avoid the crash, and allow rawCaloEnergy & rawHcalEnergy to be properly stored for packed candidates.

[2.1] 1e3e2ec#diff-49ac77e92d14b3c79dfe08879ad1b7f4054eb34bf175e844010e4a77c0d242f1
[2.2] https://github.com/cms-sw/cmssw/blob/master/RecoParticleFlow/PFProducer/plugins/ChargedHadronPFTrackIsolationProducer.cc#L53-L77

PR validation:

Check in a manual workflow with --customise RecoHGCal/TICL/iterativeTICL_cff.injectTICLintoPF
that rawCaloEnergy & rawHcalEnergy are stored in packed charged candidates. Example:

                                             (rawCaloFraction(), rawHcalFraction(), caloFraction(), hcalFraction(), ptTrk())
pfcands 2091: pt   3.2 eta -2.44 pdgId  -211 1.000 0.020 1.000 0.020 1.201 
pfcands 2092: pt   3.3 eta -2.52 pdgId   211 0.990 0.210 0.990 0.210 1.467 
pfcands 2093: pt   3.5 eta -2.84 pdgId  -211 0.990 0.380 0.990 0.380 1.616 
pfcands 2094: pt   3.8 eta -2.90 pdgId  -211 1.000 0.380 1.000 0.380 2.922 

(raw)CaloFraction() is ~1 by definition now, because PFTICL candidates don’t really use the track pt information yet, and it comes from hgcal measurements.
ptTrk is stored, so I think we have necessary information stored.

Also, make sure ttbar 2021 and 2026D49 (without injectTICLintoPF, 11634.0 & 23234.0) run without a crash.

if this PR is a backport please specify the original PR and why you need to backport that PR:

This is not a backport.

@hqucms @rovere @felicepantaleo @bendavid

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-32202/19942

• This PR adds an extra 12KB to repository

[cmsbuild]

A new Pull Request was created by @hatakeyamak (Kenichi Hatakeyama) for master.

It involves the following packages:

RecoParticleFlow/PFProducer

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@mmarionncern, @lgray, @seemasharmafnal, @cbernet this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

[slava77]

@cmsbuild please test

[cmsbuild]

The tests are being triggered in jenkins.

• CMSSW_11_2_X_2020-11-19-2300/slc7_amd64_gcc820: https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/10888/console Started: 2020/11/20 14:11

[cmsbuild]

+1
Tested at: 70faba9
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-47ea9a/10888/summary.html
CMSSW: CMSSW_11_2_X_2020-11-19-2300
SCRAM_ARCH: slc7_amd64_gcc820

[cmsbuild]

Comparison job queued.

[cmsbuild]

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

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

• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-47ea9a/11624.911_TTbar_13+2021_DD4hep+TTbar_13TeV_TuneCUETP8M1_GenSim+Digi+Reco+HARVEST+ALCA
• /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-47ea9a/11642.911_ZMM_13+2021_DD4hep+ZMM_13TeV_TuneCUETP8M1_GenSim+Digi+Reco+HARVEST+ALCA

Comparison Summary:

• No significant changes to the logs found
• Reco comparison results: 4 differences found in the comparisons
• DQMHistoTests: Total files compared: 37
• DQMHistoTests: Total histograms compared: 2962300
• DQMHistoTests: Total failures: 7
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 2962271
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
• Checked 158 log files, 34 edm output root files, 37 DQM output files

[hatakeyamak]

So this doesn't make any difference to existing workflow as expected.
I think this will need to be backported to 11_1 to make TICL injection work.
Anything we want to see before going ahead?

[perrotta]

+1

• The fix is effective for the charged pfTICL candidates
• They are all automatically considered as being "isolated": in any case, charged isolation is said not to be relevant for PF candidates from TICL
• Jenkins tests pass and show no differences in standard workflows (i.e. the ones for which injectTICLintoPF is not specified)

[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 will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[perrotta]

assign upgrade

[cmsbuild]

New categories assigned: upgrade

@kpedro88 you have been requested to review this Pull request/Issue and eventually sign? Thanks

[silviodonato]

kind reminder @cms-sw/upgrade-l2

[hatakeyamak]

I think we need this fix for TICL injection to PF work in 11_1 and 11_2 (not enabled by default, but it's being tested by various people) in regular offline reco setting (but probably not important for HLT workflow). So, this could be a candidate for backport to both 11_1 and 11_2.

[kpedro88]

+upgrade

[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 will now be reviewed by the release team before it's merged. @silviodonato, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

[silviodonato]

+1

[hatakeyamak]

Am I correct that, if we want this in 11_2, we need to make a backport request?
It's backported to 11_1 (#32316).

[kpedro88]

@hatakeyamak indeed, it might be worth making a separate backport to 11_2_X

[hatakeyamak]

ok. will do.