Update RecoParticleFlow eventcontent format and remove drop #29470
Conversation
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29470/14673
|
|
A new Pull Request was created by @jeongeun (JeongEun Lee) for master. It involves the following packages: RecoParticleFlow/Configuration @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Comparison job queued. |
| 'keep *_chargedHadronPFTrackIsolation_*_*' | ||
| ) | ||
| ) | ||
|
|
||
| # AOD content | ||
| RecoParticleFlowAOD = cms.PSet( | ||
| outputCommands = cms.untracked.vstring( | ||
| 'drop CaloTowersSorted_towerMakerPF_*_*', |
There was a problem hiding this comment.
Is this drop really needed? Even before any keep statement for it?
There was a problem hiding this comment.
No, it isn't really used so it can be deleted. There is no difference after remove this drop line.
(The towerMakerPF doesn't appear to event contents.)
| 'keep recoCaloClusters_particleFlowEGamma_*_*', | ||
| 'keep recoSuperClusters_particleFlowEGamma_*_*', | ||
| 'keep recoCaloClusters_particleFlowSuperClusterECAL_*_*', | ||
| 'keep recoSuperClusters_particleFlowSuperClusterECAL_*_*', | ||
| 'keep recoConversions_particleFlowEGamma_*_*', | ||
| 'keep recoPFCandidates_particleFlow_*_*', | ||
| 'keep recoPFCandidates_particleFlowTmp_*_*', | ||
| 'drop recoPFCandidates_particleFlowTmp__*', | ||
| 'keep recoPFCandidates_particleFlowTmp_AddedMuonsAndHadrons_*', |
There was a problem hiding this comment.
@hatakeyamak @bendavid please confirm that this list is exhaustive of what previously defined with
'keep recoPFCandidates_particleFlowTmp_*_*',
'drop recoPFCandidates_particleFlowTmp__*',
| ) | ||
|
|
||
| def _modifyPFEventContentForHGCalRECO( obj ): |
There was a problem hiding this comment.
Ok, these two def's do not seem to be used anywere
| 'keep *_particleFlow_electrons_*', | ||
| 'keep *_particleFlow_photons_*', | ||
| 'keep *_particleFlow_muons_*', | ||
| 'drop *_pfElectronTranslator_*_*', #please check pf*Translator really use in AOD |
There was a problem hiding this comment.
This is probably not needed, as all the corresponding keep statements are only after it
|
Comparison is ready Comparison Summary:
|
|
@jeongeun @hatakeyamak @bendavid please have a look at @perrotta's comments above |
|
I have to do a better job to notice when my name is mentioned in PR conversations. As far as I know, towerMakerPF is indeed not used any longer anywhere. As @abdoulline wrote #27767, it looks like it was used as inputs to PF clusters, before PF rechits started to be used? I have to admit, I don't know all the history, but it seem it's pretty safe to drop it as already done in this PR. Maybe @jeongeun is kind enough to also remove |
|
The code-checks are being triggered in jenkins. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-29470/14866
|
|
please test |
|
The tests are being triggered in jenkins. |
|
+1 |
|
Comparison job queued. |
|
Comparison is ready Comparison Summary:
|
|
+1
|
cmsbuild
added
fully-signed
reconstruction-approved
and removed
pending-signatures
reconstruction-pending
labels
|
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 (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
Update event content definitions to explicitly have RECO to be a superset of AOD and for FEVT to be a superset of RECO. Clean up 'drop' not really used in RecoParticleFlow. We use
7 'keep commands'with specific label names(L28-34) instead of'drop recoPFCandidates_particleFlowTmp__*',.In RecoParticleFlowAOD(L38-47), there is a
'drop *_pfElectronTranslator_*_*',and some'keep reco*_pfElectronTranslator_*_*',`` 'keep reco*_pfPhotonTranslator_*_*',that need check from experts more clearly.For current runTheMatrix outputs don't have
pf*Translatorin any (AOD/RECO/FEVT) event content.So please check this pf*Translator really use or need in AOD event content.
(The previous tasks are PR#29025, PR#29158, PR#29225, and PR#29299.)
PR validation:
Event Content comparison check was also done and there is no change with these updates.
Tested in CMSSW_11_1_X, the basic test all passed in the CMSSW PR instructions.