Add more calorimeter energy fractions to PackedCandidates #26506

steggema · 2019-04-23T11:50:37Z

PR description:

This PR adds two new data members to PackedCandidates to be able to save two calorimeter energy fractions (one related to ECAL over HCAL energy, the other the ratio of calorimetric energy to overall candidate energy so that absolute HCAL energy fractions as well as track-ECAL energy ratios can be calculated) to charged PF candidates in MiniAOD. The additional information is the main missing information to be able to run algorithms to reject electrons and/or muons misreconstructed as taus on taus reconstructed from MiniAOD information only (see #22594).

The PR is expected to increase the size of the MiniAOD output by 1.1%, as measured on a 10000-event TTW file from the Autumn 18 production. The size of the pat::PackedCandidates branch is expected to increase from

Branch Name | Average Uncompressed Size (Bytes/Event) | Average Compressed Size (Bytes/Event)
patPackedCandidates_packedPFCandidates__PAT. 141015 20723.3

to

patPackedCandidates_packedPFCandidates__PAT. 145227 21500.8

The proposed implementation adds two new data members so that we end up with two data members that hold "raw" information and two that hold regular calibrated information. The reason is that JetMET currently also uses the regular energy fraction data member to save raw energy fractions for isolated charged hadrons so that they can run the charged hadron calibration on MiniAOD events. I found no traces of usage in CMSSW (except for b-tagging, who use the energy fractions saved for neutrals as was the original purpose of these members) and hence propose to move the JetMET content to the raw data mebers so we can save the new information in a logically consistent way in the existing data members (@ahinzmann @zdemirag this may affect you).

@mverzett @kskovpen As far as I understand the BTV code, the DNN-based b-taggers (who seem to be the only customers in CMSSW) only use the information saved for neutrals, which are unchanged.

This is to address #26406 (see also the discussion therein). @peruzzim @rmanzoni @mbluj @slava77 @fgolf @swozniewski

PR validation:

Basic PR tests have been performed.

if this PR is a backport please specify the original PR:

N/A

…cording producer

cmsbuild · 2019-04-23T11:51:16Z

The code-checks are being triggered in jenkins.

cmsbuild · 2019-04-23T11:57:30Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-26506/9368

This PR adds an extra 48KB to repository

cmsbuild · 2019-04-23T11:58:04Z

A new Pull Request was created by @steggema (Jan Steggemann) for master.

It involves the following packages:

DataFormats/PatCandidates
PhysicsTools/PatAlgos

@perrotta, @cmsbuild, @santocch, @slava77 can you please review it and eventually sign? Thanks.
@TaiSakuma, @gouskos, @hatakeyamak, @rappoccio, @HeinerTholen, @peruzzim, @seemasharmafnal, @mmarionncern, @ahinzmann, @smoortga, @acaudron, @jdolen, @drkovalskyi, @ferencek, @rovere, @jdamgov, @nhanvtran, @gkasieczka, @schoef, @clelange, @JyothsnaKomaragiri, @mverzett, @cbernet, @gpetruc, @mariadalfonso, @pvmulder this is something you requested to watch as well.
@davidlange6, @slava77, @fabiocos you are the release manager for this.

cms-bot commands are listed here

slava77 · 2019-04-23T12:20:11Z

@cmsbuild please test

ahinzmann · 2019-04-23T12:20:38Z

Just to make sure I read the description and code correctly. This will keep saving for all isolated charged hadrons, the raw energy fractions, just under a different (+better) name, right?

cmsbuild · 2019-04-23T12:20:43Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-any-integration/34298/console Started: 2019/04/23 14:21

slava77 · 2019-04-23T12:21:09Z

assign xpog
to sign off on the size increase and perhaps double-check the code changes as well

cmsbuild · 2019-04-23T12:21:15Z

New categories assigned: xpog

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

steggema · 2019-04-23T13:14:27Z

Just to make sure I read the description and code correctly. This will keep saving for all isolated charged hadrons, the raw energy fractions, just under a different (+better) name, right?

Yes, this is the intention.

cmsbuild · 2019-04-23T16:04:45Z

-1

Tested at: ebfef1b

You can see the results of the tests here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-26506/34298/summary.html

I found follow errors while testing this PR

Failed tests: RelVals

RelVals:

When I ran the RelVals I found an error in the following workflows:
1000.0 step3

runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step3_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log

1001.0 step2

runTheMatrix-results/1001.0_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5/step2_RunMinBias2011A+RunMinBias2011A+TIER0EXP+ALCAEXP+ALCAHARVDSIPIXELCALRUN1+ALCAHARVD1+ALCAHARVD2+ALCAHARVD3+ALCAHARVD4+ALCAHARVD5.log

cmsbuild · 2019-04-23T16:04:51Z

Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped)

slava77 · 2019-05-06T17:46:57Z

PhysicsTools/PatAlgos/plugins/PATPackedCandidateProducer.cc

+            outPtrP->back().setTrackProperties(
+                *ctrack, covariancePackingSchemas_[2], covarianceVersion_);  //low quality, with pixels
+          else
+            outPtrP->back().setTrackProperties(


@davidlange6 @fabiocos
is there a way to add braces in clang-format for the cases of multi-line break-up of an if/else?
The result looks like a bit more dangerous pattern where and else can get misplaced

no - clang-format changes whitespace only. There is a clang-tidy option to add braces for ifs/fors/whiles that has been tried but so far doesn't work fully in cmssw (afaik).

slava77 · 2019-05-06T18:05:52Z

PhysicsTools/PatAlgos/plugins/PATPackedCandidateProducer.cc

 #include "RecoVertex/VertexPrimitives/interface/ConvertToFromReco.h"
+#include
+"TrackingTools/GeomPropagators/interface/AnalyticalImpactPointExtrapolator.h"


@davidlange6 @fabiocos
this is apparently breaking the commented out code

slava77 · 2019-05-06T18:09:03Z

PhysicsTools/PatAlgos/plugins/PATPackedCandidateProducer.cc

+          !iConfig.getParameter<edm::InputTag>("PuppiSrc").encode().empty() ||
+          !iConfig.getParameter<edm::InputTag>("PuppiNoLepSrc")
+               .encode()
+               .empty()),


@davidlange6 @fabiocos
for this and similar cases is there a way to make clang-format not break up similarly looking lines in a different way?
it seems also unnecessary to have .encode().empty() broken up to two pretty short lines. Was it intended in the configuration

there are not an infinite number of options, no. This presumably happens because of the greater number of characters in the second line.

One can try by hand just to have a line break before .encode() on both lines (but that also may get changed by clang-format)

cmsbuild · 2019-05-06T18:47:31Z

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

Comparison Summary:

No significant changes to the logs found
Reco comparison results: 24 differences found in the comparisons
DQMHistoTests: Total files compared: 33
DQMHistoTests: Total histograms compared: 3211964
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3211759
DQMHistoTests: Total skipped: 204
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 32 files compared)
Checked 137 log files, 14 edm output root files, 33 DQM output files

slava77 · 2019-05-06T20:05:40Z

+1

for #26506 3120396

code changes are in line with the PR description and the follow up review. Some effort was made to preserve the old treatment of hcalFraction in the deep jet/b-tagging code.
jenkins tests pass and comparisons with the baseline show differences only in the hcalFraction values, which are now also filled for most of the charged hadrons.
- deep jet/b-tagging behavior is unchanged

peruzzim · 2019-05-07T07:54:12Z

+xpog

perrotta · 2019-05-07T08:14:35Z

@davidlange6 here and in other recent PRs (see #26606 for another example) line breaking as implemented by clang mostly results in less easily readable code; moreover, it complicates the reviews quite a lot. Given my recent experiences I would simply avoid forcing all line breaks in the code checks resolution, unless some more clever setting can be found for them.

davidlange6 · 2019-05-07T08:23:48Z

I guess the review/discussions over the last year disagrees with that conclusion. But you are welcome to make a specific proposal to the core group.

…

On May 7, 2019, at 10:14 AM, perrotta ***@***.***> wrote: @davidlange6 here and in other recent PRs (see #26606 for another example) line breaking as implemented by clang mostly results in less easily readable code; moreover, it complicates the reviews quite a lot. Given my recent experiences I would simply avoid forcing all line breaks in the code checks resolution, unless some more clever setting can be found for them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

fabiocos · 2019-05-07T08:41:41Z

+1

fabiocos · 2019-05-07T08:42:43Z

merge

santocch · 2019-05-07T19:32:55Z

+1

cmsbuild · 2019-05-07T19:33:48Z

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 be automatically merged.

steggema added 2 commits April 18, 2019 16:02

Add two calorimeter energy fractions to PackedCandidates and adapt ac…

a7943af

…cording producer

Add energy fractions only for charged particles with pT > 0.5 GeV

ebfef1b

cmsbuild added this to the CMSSW_10_6_X milestone Apr 23, 2019

cmsbuild added analysis-pending code-checks-pending comparison-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 23, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 23, 2019

cmsbuild added tests-started xpog-pending and removed tests-pending labels Apr 23, 2019

cmsbuild added comparison-notrun tests-rejected and removed comparison-pending tests-started labels Apr 23, 2019

slava77 reviewed May 6, 2019

View reviewed changes

cmsbuild added comparison-available and removed comparison-pending labels May 6, 2019

cmsbuild added reconstruction-approved and removed reconstruction-pending labels May 6, 2019

cmsbuild added xpog-approved and removed xpog-pending labels May 7, 2019

cmsbuild added orp-approved and removed orp-pending labels May 7, 2019

cmsbuild merged commit 1c57cc9 into cms-sw:master May 7, 2019

cmsbuild added analysis-approved fully-signed and removed analysis-pending pending-signatures labels May 7, 2019

This was referenced Jun 11, 2019

Update of DeepTauID to ver. 2017v2 (backport to 10_2_X) #27148

Merged

Fix hcalFraction definition in DeepTauID #27173

Merged

Update of DeepTauID to ver. 2017v2 (backport for UL) #27146

Merged

mbluj mentioned this pull request Aug 27, 2020

Computation of pfCand_chHad_hcalFraction and pfCand_nHad_hcalFraction input variables in DeepTauId.cc cms-tau-pog/TauMLTools#24

Open

vlimant mentioned this pull request Oct 25, 2023

improve subdetector level hit information in PackedCandidate::bestTrack hitPattern #42412

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add more calorimeter energy fractions to PackedCandidates #26506

Add more calorimeter energy fractions to PackedCandidates #26506

steggema commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

slava77 commented Apr 23, 2019

ahinzmann commented Apr 23, 2019

cmsbuild commented Apr 23, 2019 •

edited

slava77 commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

steggema commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

slava77 May 6, 2019

davidlange6 May 7, 2019

slava77 May 6, 2019

slava77 May 6, 2019

davidlange6 May 7, 2019

cmsbuild commented May 6, 2019

slava77 commented May 6, 2019

peruzzim commented May 7, 2019

perrotta commented May 7, 2019

davidlange6 commented May 7, 2019 via email

fabiocos commented May 7, 2019

fabiocos commented May 7, 2019

santocch commented May 7, 2019

cmsbuild commented May 7, 2019

Add more calorimeter energy fractions to PackedCandidates #26506

Add more calorimeter energy fractions to PackedCandidates #26506

Conversation

steggema commented Apr 23, 2019

PR description:

PR validation:

if this PR is a backport please specify the original PR:

cmsbuild commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

slava77 commented Apr 23, 2019

ahinzmann commented Apr 23, 2019

cmsbuild commented Apr 23, 2019 • edited

slava77 commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

steggema commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

cmsbuild commented Apr 23, 2019

slava77 May 6, 2019

Choose a reason for hiding this comment

davidlange6 May 7, 2019

Choose a reason for hiding this comment

slava77 May 6, 2019

Choose a reason for hiding this comment

slava77 May 6, 2019

Choose a reason for hiding this comment

davidlange6 May 7, 2019

Choose a reason for hiding this comment

cmsbuild commented May 6, 2019

slava77 commented May 6, 2019

peruzzim commented May 7, 2019

perrotta commented May 7, 2019

davidlange6 commented May 7, 2019 via email

fabiocos commented May 7, 2019

fabiocos commented May 7, 2019

santocch commented May 7, 2019

cmsbuild commented May 7, 2019

cmsbuild commented Apr 23, 2019 •

edited