[PF] Add fillDescription to RecoParticleFlow producers #45212

stahlleiton · 2024-06-13T06:54:05Z

PR description:

This PR add the fillDescription to the producers in the RecoParticleFlow directory that were missing it. In addition, it also replaces the getByLabel with getByToken. This PR is related to the issue #43169 .

PR validation:

Tested that it compiles and ran 5 relvals locally.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

@swagata87

cmsbuild · 2024-06-13T06:54:31Z

cms-bot internal usage

cmsbuild · 2024-06-13T07:00:05Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45212/40564

This PR adds an extra 100KB to repository

cmsbuild · 2024-06-13T07:00:30Z

A new Pull Request was created by @stahlleiton for master.

It involves the following packages:

RecoParticleFlow/Configuration (reconstruction)
RecoParticleFlow/PFClusterProducer (reconstruction)
RecoParticleFlow/PFProducer (reconstruction)
RecoParticleFlow/PFSimProducer (reconstruction)
RecoParticleFlow/PFTracking (reconstruction)

@jfernan2, @cmsbuild, @mandrenguyen can you please review it and eventually sign? Thanks.
@sameasy, @lgray, @argiro, @wang0jin, @rchatter, @missirol, @hatakeyamak, @ReyerBand, @thomreis, @rovere, @felicepantaleo, @seemasharmafnal, @mmarionncern this is something you requested to watch as well.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.

cms-bot commands are listed here

swagata87 · 2024-06-13T07:40:04Z

type pf

swagata87 · 2024-06-13T07:49:46Z

@cmsbuild please test

RecoParticleFlow/Configuration/python/HepMCCopy_cfi.py

mmusich · 2024-06-13T08:12:42Z

@cms-sw/hlt-l2 FYI

mmusich · 2024-06-13T08:54:30Z

quoting from the originating issue:

In #43025 (comment), it was also noted that some of the plugins touched by #43025 do not have a fillDescriptions method (e.g. PFClusterProducer). Maybe as a result of this, I see PF code using calls to (edm::ParameterSet).exists and (edm::ParameterSet).existsAs (e.g. here), which should be dropped in favour of fillDescriptions wherever possible. Overall, the lack of fillDescriptions makes it harder to understand what is actually needed in all these (nested) PSets.

Was the removal of exists and existsAs implemented here? I don't seem to be able to find it.

mmusich · 2024-06-13T08:58:29Z

was this tested with addOnTests.py?
I see failures in HI related workflows (namely [hlt_mc_HIon:3] and [hlt_data_HIon:3]):

----- Begin Fatal Exception 13-Jun-2024 10:48:34 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFBlockProducer label='particleFlowBlock'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'useNuclear'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

mmusich · 2024-06-13T09:56:37Z

I see failures in HI related workflows (namely [hlt_mc_HIon:3] and [hlt_data_HIon:3]):

I think the problem is here:

cmssw/RecoParticleFlow/Configuration/python/RecoParticleFlow_cff.py

Line 98 in 23d93b4

e.toModify(particleFlowBlock, useNuclear = cms.bool(False))

which makes me also doubt about:

cmssw/RecoParticleFlow/PFTracking/test/fastSimWithDisplacedVertexFinder_cfg.py

Line 98 in 23d93b4

process.particleFlowBlock.useNuclear = cms.bool(True)

cmsbuild · 2024-06-13T10:12:30Z

-1

Failed Tests: RelVals RelVals-INPUT AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f69da3/39866/summary.html
COMMIT: 41f08d0
CMSSW: CMSSW_14_1_X_2024-06-12-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45212/39866/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

140.56

----- Begin Fatal Exception 13-Jun-2024 10:50:26 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFBlockProducer label='particleFlowBlock'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'useNuclear'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

312.0

----- Begin Fatal Exception 13-Jun-2024 11:00:35 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFBlockProducer label='particleFlowBlock'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'useNuclear'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

RelVals-INPUT

140.61140.61_RunHI2022/step2_RunHI2022.log
140.57140.57_RunHI2018Reduced/step2_RunHI2018Reduced.log
140.56140.56_RunHI2018/step2_RunHI2018.log

AddOn Tests

hlt_mc_HIon

----- Begin Fatal Exception 13-Jun-2024 10:48:19 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFBlockProducer label='particleFlowBlock'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'useNuclear'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

hlt_data_HIon

----- Begin Fatal Exception 13-Jun-2024 10:48:00 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFBlockProducer label='particleFlowBlock'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'useNuclear'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

Dr15Jones · 2024-06-13T12:42:48Z

RecoParticleFlow/PFClusterProducer/plugins/PFRecHitProducer.cc

-  desc.setUnknown();
-  descriptions.addDefault(desc);
+  edm::ParameterSetDescription psd;
+  psd.setUnknown();


Using setUknown really defeats the purpose of the fillDescriptions.

The issue I had in this particular class was that there were so many variations of this parameter set (in different python configuration files scattered across CMSSW), that failed to find a super set that cover all (so end up declaring it as unknown). Same with PFClusterProducer. I would have to think if there is a way to avoid having a long list of optional parameters.

The unknown parameter set descriptions are removed in 2da72f0

@stahlleiton just a word of caution about the (many) addOptional added in 2da72f0. Currently the ConfDB parsing skips any cms.optional in the resulting cfi files, so if at some point any of these need to be configured at HLT it won't be possible (unless a default is provided in the C++ source code).

I made them optional because there were parameter sets with different content used in different parts of CMSSW. So I only used add for those parameters that were common among all parameter sets.

Removed the optional condition from all the parameters I added in the fillDescription in c85415b

RecoParticleFlow/PFClusterProducer/plugins/PFClusterProducer.cc

stahlleiton · 2024-06-13T13:57:20Z

quoting from the originating issue:

In #43025 (comment), it was also noted that some of the plugins touched by #43025 do not have a fillDescriptions method (e.g. PFClusterProducer). Maybe as a result of this, I see PF code using calls to (edm::ParameterSet).exists and (edm::ParameterSet).existsAs (e.g. here), which should be dropped in favour of fillDescriptions wherever possible. Overall, the lack of fillDescriptions makes it harder to understand what is actually needed in all these (nested) PSets.

Was the removal of exists and existsAs implemented here? I don't seem to be able to find it.

Not yet, and in this PR I only added the fill descriptions and was planning to fix the "exist" and "existAs" in another PR.

stahlleiton · 2024-06-13T13:58:13Z

was this tested with addOnTests.py?
I see failures in HI related workflows (namely [hlt_mc_HIon:3] and [hlt_data_HIon:3]):

----- Begin Fatal Exception 13-Jun-2024 10:48:34 CEST-----------------------
An exception of category 'Configuration' occurred while
   [0] Constructing the EventProcessor
   [1] Validating configuration of module: class=PFBlockProducer label='particleFlowBlock'
Exception Message:
Illegal parameter found in configuration.  The parameter is named:
 'useNuclear'
You could be trying to use a parameter name that is not
allowed for this plugin or it could be misspelled.
----- End Fatal Exception -------------------------------------------------

I tested it with pp and UPC relvals, forgot to also include HI relvals. Will fix and test again

cmsbuild · 2024-06-18T19:41:45Z

Pull request #45212 was updated. @mandrenguyen, @Martin-Grunewald, @cmsbuild, @jfernan2, @mmusich can you please check and sign again.

…ctors

cmsbuild · 2024-06-19T00:50:24Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45212/40644

This PR adds an extra 80KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File RecoParticleFlow/PFRecHitProducer/test/PFRecHitProducerTest.cc modified in PR(s): [RFC] descriptions.addDefault -> descriptions.addWithDefaultLabel #44372

cmsbuild · 2024-06-19T00:50:51Z

Pull request #45212 was updated. @mandrenguyen, @cmsbuild, @jfernan2 can you please check and sign again.

mmusich · 2024-06-19T06:23:31Z

@cmsbuild, please test

cmsbuild · 2024-06-19T10:02:18Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f69da3/39965/summary.html
COMMIT: 9fee0bb
CMSSW: CMSSW_14_1_X_2024-06-18-1800/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/45212/39965/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 2 lines to the logs
Reco comparison results: 1 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3345018
DQMHistoTests: Total failures: 3
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3344995
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

stahlleiton · 2024-06-20T09:21:39Z

Are there further comments on this PR?

mandrenguyen · 2024-06-20T16:37:12Z

+1

cmsbuild · 2024-06-20T16:37:35Z

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. @antoniovilela, @sextonkennedy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

antoniovilela · 2024-06-21T00:11:41Z

+1

Add fillDescription to RecoParticleFlow producers

41f08d0

cmsbuild added this to the CMSSW_14_1_X milestone Jun 13, 2024

cmsbuild added reconstruction-pending pending-signatures tests-pending orp-pending code-checks-pending labels Jun 13, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 13, 2024

cmsbuild added the pf label Jun 13, 2024

swagata87 mentioned this pull request Jun 13, 2024

fillDescriptions for all PFlow producers and better handling of HcalPFCuts #43169

Open

cmsbuild added tests-started and removed tests-pending labels Jun 13, 2024

mmusich reviewed Jun 13, 2024

View reviewed changes

RecoParticleFlow/Configuration/python/HepMCCopy_cfi.py Show resolved Hide resolved

cmsbuild added tests-rejected and removed tests-started labels Jun 13, 2024

Dr15Jones reviewed Jun 13, 2024

View reviewed changes

mmusich reviewed Jun 13, 2024

View reviewed changes

RecoParticleFlow/PFClusterProducer/plugins/PFClusterProducer.cc Outdated Show resolved Hide resolved

cmsbuild added the code-checks-approved label Jun 18, 2024

Undo the renaming of gathering, seeding and reHitEnergyNorm double ve…

9fee0bb

…ctors

cmsbuild added code-checks-pending and removed hlt-pending code-checks-approved labels Jun 19, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 19, 2024

cmsbuild added tests-started and removed tests-pending labels Jun 19, 2024

cmsbuild added tests-approved and removed tests-started labels Jun 19, 2024

cmsbuild added reconstruction-approved fully-signed and removed reconstruction-pending pending-signatures labels Jun 20, 2024

cmsbuild added orp-approved and removed orp-pending labels Jun 21, 2024

cmsbuild merged commit 40b9d3c into cms-sw:master Jun 21, 2024
11 checks passed

stahlleiton deleted the PF_CMSSW_14_1_X branch June 21, 2024 03:34

cmsbuild mentioned this pull request Jun 21, 2024

Heterogeneous Workflow (CLUE) in Phase2 HLT Simplified Menu #45178

Merged

swagata87 mentioned this pull request Jul 27, 2024

[14_0_X] Reject badly measured tracks with very high values of qoverpError before trying to reconstruct displaced vertices #45569

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PF] Add fillDescription to RecoParticleFlow producers #45212

[PF] Add fillDescription to RecoParticleFlow producers #45212

stahlleiton commented Jun 13, 2024

cmsbuild commented Jun 13, 2024 •

edited

Loading

cmsbuild commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

swagata87 commented Jun 13, 2024

swagata87 commented Jun 13, 2024

mmusich commented Jun 13, 2024

mmusich commented Jun 13, 2024

mmusich commented Jun 13, 2024 •

edited

Loading

mmusich commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

Dr15Jones Jun 13, 2024

stahlleiton Jun 13, 2024 •

edited

Loading

stahlleiton Jun 16, 2024

mmusich Jun 16, 2024 •

edited

Loading

stahlleiton Jun 16, 2024

stahlleiton Jun 18, 2024

stahlleiton commented Jun 13, 2024 •

edited

Loading

stahlleiton commented Jun 13, 2024

cmsbuild commented Jun 18, 2024

cmsbuild commented Jun 19, 2024

cmsbuild commented Jun 19, 2024

mmusich commented Jun 19, 2024

cmsbuild commented Jun 19, 2024

stahlleiton commented Jun 20, 2024

mandrenguyen commented Jun 20, 2024

cmsbuild commented Jun 20, 2024

antoniovilela commented Jun 21, 2024

[PF] Add fillDescription to RecoParticleFlow producers #45212

[PF] Add fillDescription to RecoParticleFlow producers #45212

Conversation

stahlleiton commented Jun 13, 2024

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

cmsbuild commented Jun 13, 2024 • edited Loading

cmsbuild commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

swagata87 commented Jun 13, 2024

swagata87 commented Jun 13, 2024

mmusich commented Jun 13, 2024

mmusich commented Jun 13, 2024

mmusich commented Jun 13, 2024 • edited Loading

mmusich commented Jun 13, 2024

cmsbuild commented Jun 13, 2024

RelVals

RelVals-INPUT

AddOn Tests

Dr15Jones Jun 13, 2024

Choose a reason for hiding this comment

stahlleiton Jun 13, 2024 • edited Loading

Choose a reason for hiding this comment

stahlleiton Jun 16, 2024

Choose a reason for hiding this comment

mmusich Jun 16, 2024 • edited Loading

Choose a reason for hiding this comment

stahlleiton Jun 16, 2024

Choose a reason for hiding this comment

stahlleiton Jun 18, 2024

Choose a reason for hiding this comment

stahlleiton commented Jun 13, 2024 • edited Loading

stahlleiton commented Jun 13, 2024

cmsbuild commented Jun 18, 2024

cmsbuild commented Jun 19, 2024

cmsbuild commented Jun 19, 2024

mmusich commented Jun 19, 2024

cmsbuild commented Jun 19, 2024

Comparison Summary

stahlleiton commented Jun 20, 2024

mandrenguyen commented Jun 20, 2024

cmsbuild commented Jun 20, 2024

antoniovilela commented Jun 21, 2024

cmsbuild commented Jun 13, 2024 •

edited

Loading

mmusich commented Jun 13, 2024 •

edited

Loading

stahlleiton Jun 13, 2024 •

edited

Loading

mmusich Jun 16, 2024 •

edited

Loading

stahlleiton commented Jun 13, 2024 •

edited

Loading