PFCandidate no longer inherits from CompositeCandidate by Dr15Jones · Pull Request #38999 · cms-sw/cmssw

Dr15Jones · 2022-08-08T17:41:48Z

PR description:

The storing of PFCandidates takes 1/3rd of the entire time it takes to write AOD. I have started to look at the class to see if there was a way to make it easier for ROOT to store. The first think I tried to was to remove CompositeCandidate (as that looks hard to store) in order to see what would break and therefore would have to be changed if I removed the inheritance. To my surprise no code in CMSSW made use of the possible compositeness of the class.

PR validation:

The Code and all dependent code compiles.

No code in CMSSW made use of the possible compositeness of the class.

cmsbuild · 2022-08-08T17:50:10Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38999/31468

This PR adds an extra 32KB to repository

cmsbuild · 2022-08-08T17:50:33Z

A new Pull Request was created by @Dr15Jones (Chris Jones) for master.

It involves the following packages:

DataFormats/ParticleFlowCandidate (reconstruction)
DataFormats/PatCandidates (reconstruction)

@jpata, @cmsbuild, @clacaputo can you please review it and eventually sign? Thanks.
@gouskos, @rovere, @lgray, @missirol, @hatakeyamak, @gpetruc this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2022-08-08T18:00:56Z

please test

Dr15Jones · 2022-08-08T21:06:43Z

I did a test where I wrote a 10,000 event file containing only recoPFCandidates_particleFlow__RECO using standard AOD settings. If I wrote the file with no compression, this PR made the output module run 40% faster. If I used the standard AOD compression, the times were the same within measurement error (just 1% difference).

cmsbuild · 2022-08-08T21:57:42Z

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d5d55e/26710/summary.html
COMMIT: 85e0542
CMSSW: CMSSW_12_5_X_2022-08-08-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38999/26710/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found errors in the following unit tests:

---> test runtestTqafTopJetCombination had ERRORS
---> test runtestTqafTopEventSelection had ERRORS
---> test runtestTqafTopTools had ERRORS
---> test runtestTqafTopHitFit had ERRORS
and more ...

RelVals

136.8311136.8311_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017/step2_RunJetHT2017F_reminiaod+RunJetHT2017F_reminiaod+REMINIAOD_data2017+HARVEST2017_REMINIAOD_data2017.log
136.7611136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log
136.88811136.88811_RunJetHT2018D_reminiaodUL+RunJetHT2018D_reminiaodUL+REMINIAOD_data2018UL+HARVEST2018_REMINIAOD_data2018UL/step2_RunJetHT2018D_reminiaodUL+RunJetHT2018D_reminiaodUL+REMINIAOD_data2018UL+HARVEST2018_REMINIAOD_data2018UL.log

Expand to see more relval errors ...

158.01

RelVals-INPUT

136.72411136.72411_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM/step2_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMINIAOD_data2016UL_HIPM+HARVESTDR2_REMINIAOD_data2016UL_HIPM.log
136.72412136.72412_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMININANO_data2016UL_HIPM+HARVESTDR2_REMININANO_data2016UL_HIPM/step2_RunJetHT2016B_reminiaodUL+RunJetHT2016B_reminiaodUL+REMININANO_data2016UL_HIPM+HARVESTDR2_REMININANO_data2016UL_HIPM.log
136.7611136.7611_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM/step2_RunJetHT2016E_reminiaod+RunJetHT2016E_reminiaod+REMINIAOD_data2016_HIPM+HARVESTDR2_REMINIAOD_data2016_HIPM.log

Expand to see more relval errors ...

AddOn Tests

pat1cmsRun /data/cmsbld/jenkins/workspace/ib-run-pr-addon/CMSSW_12_5_X_2022-08-08-1100/src/PhysicsTools/PatAlgos/test/IntegrationTest_cfg.py : FAILED - time: date Mon Aug 8 21:01:15 2022-date Mon Aug 8 20:59:52 2022 s - exit: 35584

Dr15Jones · 2022-08-09T14:09:40Z

@pcanal So I changed the inheritance for reco::PFCandidate, dropping one of the intermediate base classes. When I did that I updated all the class versions as well. However, now the tests run for the PR are failing where all these tests seem to be reading back reco::PFCandidates which had been stored using the previous version. It seems like the scheme evolution isn't working properly.

Dr15Jones · 2022-08-09T16:03:43Z

So I wrote a simple EDAnalyzer which dumps the contents of a std::vector<reco::PFCandidate>. I ran it using both the original code and the new code, both reading the same input file (which was written using the original format). The results are

element 0	PFCandidate type: 1 E/pT/eta/phi 0.811/0.175/-2.196/-0.403, blocks/ie |	element 0	PFCandidate type: 1 E/pT/eta/phi 0.000/0.175/-2.196/-0.403, blocks/ie
element 1	PFCandidate type: 1 E/pT/eta/phi 1.500/0.230/2.559/2.968, blocks/iele |	element 1	PFCandidate type: 1 E/pT/eta/phi 0.000/0.230/2.559/2.968, blocks/iele
element 2	PFCandidate type: 1 E/pT/eta/phi 0.589/0.180/1.825/-2.992, blocks/iel |	element 2	PFCandidate type: 1 E/pT/eta/phi 0.000/0.180/1.825/-2.992, blocks/iel
element 3	PFCandidate type: 1 E/pT/eta/phi 1.244/0.205/2.484/-3.059, blocks/iel |	element 3	PFCandidate type: 1 E/pT/eta/phi 0.000/0.205/2.484/-3.059, blocks/iel
element 4	PFCandidate type: 1 E/pT/eta/phi 9.328/2.029/-2.206/-1.708, blocks/ie |	element 4	PFCandidate type: 1 E/pT/eta/phi 0.000/2.029/-2.206/-1.708, blocks/ie
element 5	PFCandidate type: 1 E/pT/eta/phi 2.963/0.630/-2.229/-0.503, blocks/ie |	element 5	PFCandidate type: 1 E/pT/eta/phi 0.000/0.630/-2.229/-0.503, blocks/ie

So when reading back using the new format, energy is always 0 while pT, eta, and phi look fine.

pcanal · 2022-08-09T16:15:23Z

Which data member does the energy and pT (or eta or phi) correspond to?

Dr15Jones · 2022-08-09T16:29:12Z

So PFCandidate::energy() is read from LeafCandidate::m_state:: p4Cartesian_ where that member variable is declared transient to ROOT.

cmssw/DataFormats/Candidate/src/classes_def.xml

Line 7 in e52f99a

the value is supposed to be filled by the IO rule

cmssw/DataFormats/Candidate/src/classes_def.xml

Lines 9 to 12 in e52f99a

    
           <!--The 'int pdgId_' is a workaround for ROOT bug which affects iorules for split classes--> 
        
           <ioread sourceClass = "reco::ParticleState" version="[1-]" targetClass="reco::ParticleState" source="ROOT::Math::LorentzVector<ROOT::Math::PtEtaPhiM4D<double> > p4Polar_;int pdgId_" target="p4Cartesian_"> 
        
            <![CDATA[newObj->setCartesian();]]> 
        
           </ioread>

So it appears that when schema evolution is being used, that the necessary IO rules are not always being run.

Dr15Jones · 2022-08-09T16:34:02Z

So pT, eta and phi come from LeafCandidate::m_state::p4Polar_ which is stored.

Dr15Jones · 2022-08-09T16:38:34Z

So if I do

[cdj@cmslpc-c8-heavy01 matrix]$ root -l root://eoscms.cern.ch//eos/cms/store/user/cmsbuild/store/data/Run2017F/JetHT/AOD/17Nov2017-v1/70000/0054D144-36DF-E711-B074-02163E01A47B.root
root [1] Events->Scan("recoPFCandidates_particleFlow__RECO.obj.m_state.energy()")

using the original format I get

***********************************
*    Row   * Instance * recoPFCan *
***********************************
*        0 *        0 * 0.8107478 *
*        0 *        1 * 1.5003893 *
*        0 *        2 * 0.5893012 *
*        0 *        3 * 1.2443912 *
*        0 *        4 * 9.3277593 *
*        0 *        5 * 2.9629352 *
*        0 *        6 * 3.7514724 *
*        0 *        7 * 2.8305396 *
*        0 *        8 * 1.6818398 *
*        0 *        9 * 1.7144627 *
*        0 *       10 * 0.9533503 *
*        0 *       11 * 7.2343502 *
*        0 *       12 * 5.6262460 *
*        0 *       13 * 0.9747063 *
*        0 *       14 * 9.3023527 *
*        0 *       15 * 1.4282982 *
*        0 *       16 * 4.3854562 *
*        0 *       17 * 1.2034772 *
*        0 *       18 * 1.9004409 *
*        0 *       19 * 1.6507877 *
*        0 *       20 * 1.9658588 *
*        0 *       21 * 1.9287844 *
*        0 *       22 * 2.9119759 *
*        0 *       23 * 0.7939654 *
*        0 *       24 * 0.6640725 *

but using the new format I get

***********************************
*    Row   * Instance * recoPFCan *
***********************************
*        0 *        0 *         0 *
*        0 *        1 *         0 *
*        0 *        2 * 2.84e-306 *
*        0 *        3 *         0 *
*        0 *        4 *         0 *
*        0 *        5 *         0 *
*        0 *        6 *         0 *
*        0 *        7 *         0 *
*        0 *        8 * 2.84e-306 *
*        0 *        9 * 2.84e-306 *
*        0 *       10 * 2.84e-306 *
*        0 *       11 *         0 *
*        0 *       12 *         0 *
*        0 *       13 * 2.84e-306 *
*        0 *       14 *         0 *
*        0 *       15 * 2.84e-306 *
*        0 *       16 *         0 *
*        0 *       17 * 2.84e-306 *
*        0 *       18 *         0 *
*        0 *       19 * 2.84e-306 *
*        0 *       20 *         0 *
*        0 *       21 * 2.84e-306 *
*        0 *       22 *         0 *
*        0 *       23 * 3.72e-307 *
*        0 *       24 * 2.84e-306 *

Dr15Jones · 2022-08-09T16:40:24Z

I'm doing this using CMSSW_12_5_0_pre4 which is using a version of ROOT 6.24.07

pcanal · 2022-08-09T16:50:50Z

Fair enough. It seems to have to do with the handling/passing-through to the base class. Let me try to write a standalone reproducer.

pcanal · 2022-08-10T16:07:09Z

I can reproduce locally. I am looking for a solution.

cmsbuild · 2025-02-07T08:31:50Z

Milestone for this pull request has been moved to CMSSW_15_1_X. Please open a backport if it should also go in to CMSSW_15_0_X.

cmsbuild · 2025-02-07T10:01:23Z

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d5d55e/44258/summary.html
COMMIT: 85e0542
CMSSW: CMSSW_15_0_X_2025-02-06-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38999/44258/install.sh to create a dev area with all the needed externals and cmssw changes.

Unit Tests

I found 8 errors in the following unit tests:

---> test runtestPhysicsToolsPatAlgos had ERRORS
---> test testRecoMETMETProducers had ERRORS
---> test runtestTqafTopEventSelection had ERRORS
and more ...

RelVals

2500.201

----- Begin Fatal Exception 07-Feb-2025 10:34:43 CET-----------------------
An exception of category 'FileReadError' occurred while
   [0] Processing  Event run: 1 lumi: 85 event: 8402 stream: 0
   [1] Running path 'dqmoffline_step'
   [2] Prefetching for module NanoAODDQM/'nanoDQMMC'
   [3] Prefetching for module TriggerObjectTableProducer/'triggerObjectTable'
   [4] While reading from source BXVector<l1t::EGamma> caloStage2Digis 'EGamma' RECO
   [5] Rethrowing an exception that happened on a different read request.
   [6] Processing  Event run: 1 lumi: 85 event: 8402 stream: 0
   [7] Running path 'dqmoffline_step'
   [8] Prefetching for module NanoAODDQM/'nanoDQMMC'
   [9] Prefetching for module CandMCMatchTableProducer/'boostedTauMCTable'
   [10] Prefetching for module PATObjectCrossLinker/'linkedObjects'
   [11] Prefetching for module PATMuonRefSelector/'finalMuons'
   [12] Prefetching for module PATMuonUserDataEmbedder/'slimmedMuonsWithUserData'
   [13] Prefetching for module PATMuonUpdater/'slimmedMuonsUpdated'
   [14] While reading from source std::vector<pat::Muon> slimmedMuons '' RECO
   [15] Reading branch patMuons_slimmedMuons__RECO.
   Additional Info:
      [a] Fatal Root Error: @SUB=TBufferFile::ReadClassBuffer
Could not find the StreamerInfo for version 11 of the class edm::RefVector<vector<reco::PFBlock>,reco::PFBlock,edm::refhelper::FindUsingAdvance<vector<reco::PFBlock>,reco::PFBlock> >, object skipped at offset 27231

----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

2500.2322500.232_EGMNANOdata140Xrun3/step2_EGMNANOdata140Xrun3.log
2500.2332500.233_BTVNANOdata140Xrun3/step2_BTVNANOdata140Xrun3.log
2500.2312500.231_muPOGNANO140Xrun3/step2_muPOGNANO140Xrun3.log

Expand to see more relval errors ...

AddOn Tests

pat1

A fatal system signal has occurred: segmentation violation

cmsbuild · 2025-09-10T06:54:37Z

Milestone for this pull request has been moved to CMSSW_16_0_X. Please open a backport if it should also go in to CMSSW_15_1_X.

cmsbuild · 2025-12-18T12:58:12Z

Milestone for this pull request has been moved to CMSSW_16_1_X. Please open a backport if it should also go in to CMSSW_16_0_X.

cmsbuild · 2026-04-04T12:39:23Z

Milestone for this pull request has been moved to CMSSW_17_0_X. Please open a backport if it should also go in to CMSSW_16_1_X.

cmsbuild · 2026-04-04T14:08:43Z

-1

Failed Tests: UnitTests RelVals RelVals-INPUT AddOn
Size: This PR adds an extra 16KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d5d55e/52470/summary.html
COMMIT: 85e0542
CMSSW: CMSSW_16_1_X_2026-04-04-1100/el8_amd64_gcc13
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38999/52470/install.sh to create a dev area with all the needed externals and cmssw changes.

Failed Unit Tests

I found 10 errors in the following unit tests:

---> test test-btvNano-run had ERRORS
---> test runtestPhysicsToolsPatAlgos had ERRORS
---> test testRecoMETMETProducers had ERRORS
and more ...

Failed RelVals

2500.3001A fatal system signal has occurred: segmentation violation

Failed RelVals-INPUT

2500.3312500.331_BPHNANOdata150Xrun3/step2_BPHNANOdata150Xrun3.log
2500.33062500.3306_lepTrackInfoNANOdata150Xrun3/step2_lepTrackInfoNANOdata150Xrun3.log
2500.22082500.2208_ScoutingNANOwithPromptmc140X/step2_ScoutingNANOwithPromptmc140X.log

Expand to see more relval errors ...

Failed AddOn Tests

pat1

A fatal system signal has occurred: segmentation violation

makortel · 2026-04-07T18:57:07Z

This PR was moved to #50682 as part of the EVOLUTION_X work

makortel · 2026-04-07T18:57:13Z

@cmsbuild, please close

PFCandidate no longer inherits from CompositeCandidate

85e0542

No code in CMSSW made use of the possible compositeness of the class.

cmsbuild added this to the CMSSW_12_5_X milestone Aug 8, 2022

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Aug 8, 2022

cmsbuild added code-checks-approved and removed code-checks-pending labels Aug 8, 2022

cmsbuild added tests-started and removed tests-pending labels Aug 8, 2022

cmsbuild added tests-rejected and removed tests-started labels Aug 8, 2022

smuzaffar modified the milestones: CMSSW_12_5_X, CMSSW_12_6_X Aug 28, 2022

cmsbuild mentioned this pull request Sep 12, 2022

Pf cand rescale #39368

Merged

cmsbuild added this to the CMSSW_15_0_X milestone Nov 22, 2024

cmsbuild modified the milestones: CMSSW_15_0_X, CMSSW_15_1_X Feb 7, 2025

cmsbuild added tests-started and removed tests-rejected labels Feb 7, 2025

cmsbuild modified the milestones: CMSSW_15_0_X, CMSSW_15_1_X Feb 7, 2025

cmsbuild added tests-rejected and removed tests-started labels Feb 7, 2025

cmsbuild mentioned this pull request Mar 22, 2025

BPHNanoAODs #47603

Merged

cmsbuild modified the milestones: CMSSW_15_1_X, CMSSW_16_0_X Sep 10, 2025

cmsbuild mentioned this pull request Nov 3, 2025

[Do not merge] LLVM21 clang-tidy changes #49297

Closed

cmsbuild mentioned this pull request Nov 26, 2025

[RECONSTRUCTION] [LLVM21] Apply clang-tidy changes #49496

Merged

cmsbuild mentioned this pull request Jan 9, 2026

Move AtomicPtrCache dictionary to proper packages #49757

Merged

cmsbuild mentioned this pull request Feb 6, 2026

Add various missing dictionaries #50065

Merged

cmsbuild mentioned this pull request Mar 4, 2026

MiniAOD from HLT Scouting #50307

Merged

makortel mentioned this pull request Apr 7, 2026

[EVO] PFCandidate no longer inherits from CompositeCandidate #50682

Merged

Conversation

Dr15Jones commented Aug 8, 2022

PR description:

PR validation:

Uh oh!

cmsbuild commented Aug 8, 2022

Uh oh!

cmsbuild commented Aug 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dr15Jones commented Aug 8, 2022

Uh oh!

Dr15Jones commented Aug 8, 2022

Uh oh!

cmsbuild commented Aug 8, 2022

Unit Tests

RelVals

RelVals-INPUT

AddOn Tests

Uh oh!

Dr15Jones commented Aug 9, 2022

Uh oh!

Dr15Jones commented Aug 9, 2022

Uh oh!

pcanal commented Aug 9, 2022

Uh oh!

Dr15Jones commented Aug 9, 2022

Uh oh!

Dr15Jones commented Aug 9, 2022

Uh oh!

Dr15Jones commented Aug 9, 2022

Uh oh!

Dr15Jones commented Aug 9, 2022

Uh oh!

pcanal commented Aug 9, 2022

Uh oh!

pcanal commented Aug 10, 2022

Uh oh!

cmsbuild commented Feb 7, 2025

Uh oh!

cmsbuild commented Feb 7, 2025

Unit Tests

RelVals

RelVals-INPUT

AddOn Tests

Uh oh!

cmsbuild commented Sep 10, 2025

Uh oh!

cmsbuild commented Dec 18, 2025

Uh oh!

cmsbuild commented Apr 4, 2026

Uh oh!

cmsbuild commented Apr 4, 2026

Failed Unit Tests

Failed RelVals

Failed RelVals-INPUT

Failed AddOn Tests

Uh oh!

makortel commented Apr 7, 2026

Uh oh!

makortel commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

cmsbuild commented Aug 8, 2022 •

edited

Loading