Add wildcards to generated _cfi.py files

[Dr15Jones]

PR description:

The first wildcard denoted in a ParameterSet descriptions will be added to the cfi.py file via the allowAnyLabel mechanic.

PR validation:

Framework unit tests pass.

[cmsbuild]

cms-bot internal usage

[Dr15Jones]

please test

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44013/38939

• This PR adds an extra 48KB to repository

[cmsbuild]

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

It involves the following packages:

• FWCore/Integration (core)
• FWCore/ParameterSet (core)

@makortel, @Dr15Jones, @smuzaffar can you please review it and eventually sign? Thanks.
@makortel, @missirol, @wddgit 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

[Dr15Jones]

@cms-sw/hlt-l2 FYI this will change the _cfi.py files for any module which declares a wildcard for a parameter label. This will now add a line to the _cfi.py configuration starting with allowAnyLabel_ = ... which naively looks like a declaration of a parameter with a label of allowAnyLabel_ but that is not the case. This might affect HLT cfi parsing logic.

[Martin-Grunewald]

Sorry, the PR description is too terse for me to understand.
What is the problem to be solved by this PR?
Do you have an example new vs old?
Thanks!

[Dr15Jones]

So previously, if the fillDescriptions contained something like

    edm::ParameterSetDescription desc;
    desc.addOptional<unsigned>("p_uint_opt", 0);
    desc.addWildcard<int>("*");

The module declaration in the _cfi.py file would only contain

     p_uint_opt = cms.uint32(0)

but with this change it would become

     p_uint_opt = cms.uint32(0),
     allowAnyLabel_ = cms.optional.int32

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0f442f/37554/summary.html
COMMIT: d809da0
CMSSW: CMSSW_14_1_X_2024-02-19-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44013/37554/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially removed 10 lines from the logs
• Reco comparison results: 37 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3248569
• DQMHistoTests: Total failures: 0
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3248547
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 200 log files, 161 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[makortel]

@Martin-Grunewald Do you have any further comments?

[Martin-Grunewald]

I do not understand the above example line

p_uint_opt = cms.uint32(0)

Shouldn't that be

p_uint_opt = cms.optional.uint32(0)

?

In any case, the line:

allowAnyLabel_ = cms.optional.int32

would be interpreted as a parameter of name allowAnyLabel_ but since cms.optional.int32 is also not understood by ConfDb parsing (since back then when it was introduced), the whole line would be ignored.
Thus if that is a top-level parameter, ConfDb would not allow it. Within a PSet, arbitrary parameters can by added in the GUI. In any case, cms.optional is not understood by parsing, would not be stored in ConfDb, and would not be put in a python config extracted from ConfDb.

So not useable at HLT, but also, so far we did not encounter an issue with the existing cms.optional (ie, ignoring seems fine?)

[Dr15Jones]

p_uint_opt = cms.uint32(0)

Shouldn't that be

p_uint_opt = cms.optional.uint32(0)

Arg, you are right. I meant to use in the example

desc.add<unsigned>("p_uint_opt", 0);

but the lines dealing with p_uint_opt are not actually important to the example, only the wildcard.

[makortel]

Thanks @Martin-Grunewald

[makortel]

+core

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

[makortel]

The last test showed the usual

• Non reproducibility in wf 136.793 #43293
• Non-reproducibility in TriggerResults in 24834.0, 24834.911, and 25034.999 #43790

[Dr15Jones]

@Martin-Grunewald on a related note, do you know how the ConfDb entries derived from _cfi.py files are actually handled? Does the code look at the _cfi.py as a text file or does it load it into python and then interrogate the python object? Thanks.

[rappoccio]

+1

[Martin-Grunewald]

@Martin-Grunewald on a related note, do you know how the ConfDb entries derived from _cfi.py files are actually handled? Does the code look at the _cfi.py as a text file or does it load it into python and then interrogate the python object? Thanks.

It loads it into python and goes from there.