Make sure to report an error if process.options.numberOfThreads has wrong type

[makortel]

PR description:

Resolves #32070

PR validation:

FWCore/Framework unit tests run.

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:

To be backported to all or some subset of "active release cycles".

• 13_0_X [13_0_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41598
• 12_6_X [12_6_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41620
• 12_5_X [12_5_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41626
• 12_4_X [12_4_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41627
• 10_6_X [10_6_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41643
• 9_4_X [9_4_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41654
• 8_0_X [8_0_X] Make sure to report an error if process.options.numberOfThreads has wrong type #41735

[cmsbuild]

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-41349/35179

• This PR adds an extra 20KB to repository

[cmsbuild]

A new Pull Request was created by @makortel (Matti Kortelainen) for master.

It involves the following packages:

• FWCore/Framework (core)
• FWCore/ParameterSet (core)

@cmsbuild, @smuzaffar, @Dr15Jones, @makortel can you please review it and eventually sign? Thanks.
@missirol, @wddgit this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

[makortel]

@cmsbuild, please test

[makortel]

@Dr15Jones please review

[cmsbuild]

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-501915/31985/summary.html
COMMIT: 0b40a7a
CMSSW: CMSSW_13_1_X_2023-04-14-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/41349/31985/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

• You potentially added 1 lines to the logs
• Reco comparison results: 6 differences found in the comparisons
• DQMHistoTests: Total files compared: 48
• DQMHistoTests: Total histograms compared: 3459609
• DQMHistoTests: Total failures: 6
• DQMHistoTests: Total nulls: 0
• DQMHistoTests: Total successes: 3459581
• DQMHistoTests: Total skipped: 22
• DQMHistoTests: Total Missing objects: 0
• DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
• Checked 207 log files, 159 edm output root files, 48 DQM output files
• TriggerResults: no differences found

[Dr15Jones]

Should we consider making the setting function more recursive? I.e. if it is already set with an 'optional' it enforces the type checking? I'm guessing we didn't do that initially as some older parts wanted to be able to declare different types. But if we made 'optional' smarter to know it could have different types, maybe we could handle that case as well.

In that way optional could always catch type problems at the python level.

[Dr15Jones]

Even if we do what I suggest above, I think we should still do the check you are doing in the PR at the C++ level as well.

[makortel]

I agree in being able to catch the problem already in python would be be better (at least for some patterns, I guess we can't do much for del process.options.numberOfThreads; process.options.numberOfThreads = cms.untracked.int64(3)).

A technical complication for stronger enforcement is that the assignment

process.options.numberOfThreads = cms.untracked.int64(3)

executes code in the process.options PSet. So the PSet.__setattr__() would have to be made smarter (it looks doable though).

Another complication is that only one parameter in the process.options PSet is optional, and the others (including numberOfThreads) have a default value set. So I think we'd need to think more how exactly to proceed.

All that sounds like a material for a new issue.

[makortel]

I opened an issue #41356

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

[perrotta]

+1