Can mark Parameters as obsolete in fillDescriptions by Dr15Jones · Pull Request #47642 · cms-sw/cmssw

Dr15Jones · 2025-03-20T15:42:45Z

PR description:

Obsolete parameters will cause a warning to be issued and then skipped in the validation.
The _cfi.py files will get a cms.obsolete parameter added which allows one to still set the parameter in python but will not be passed to the C++.

PR validation:

Framework unit tests pass and newly added tests also pass.

resolves cms-sw/framework-team#1317

cmsbuild · 2025-03-20T15:43:10Z

cms-bot internal usage

cmsbuild · 2025-03-20T15:44:21Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47642/44180

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/ParameterSet/interface/ParameterDescription.h modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/interface/ParameterDescriptionNode.h modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterDescription.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterDescriptionBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/src/ParameterDescriptionNode.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterSwitchBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/src/ParameterWildcardBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/test/test_catch_ParameterSetDescription.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47642/44180/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47642/44180/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

Dr15Jones · 2025-03-20T15:50:31Z

please test

cmsbuild · 2025-03-20T15:51:56Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47642/44182

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/ParameterSet/interface/ParameterDescription.h modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/interface/ParameterDescriptionNode.h modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterDescription.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterDescriptionBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/src/ParameterDescriptionNode.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterSwitchBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/src/ParameterWildcardBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/test/test_catch_ParameterSetDescription.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216

cmsbuild · 2025-03-20T15:52:28Z

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

It involves the following packages:

FWCore/ParameterSet (core)

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

cms-bot commands are listed here

makortel · 2025-03-20T19:19:49Z

FWCore/ParameterSet/interface/ParameterSetDescription.h

+    ParameterDescriptionBase* addObsolete(U const& iLabel) {
+      if constexpr (std::is_same_v<T, edm::ParameterSetDescription>) {


I'm wondering if a short comment here (and below for the untracked) explaining why ParameterSetDescription needs a different treatment would be useful for a future reader?

FWCore/ParameterSet/interface/ParameterSetDescription.h

FWCore/ParameterSet/test/test_catch_ParameterSetDescription.cc

cmsbuild · 2025-03-20T19:38:38Z

+1

Size: This PR adds an extra 72KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-83502d/45113/summary.html
COMMIT: 3b46cbe
CMSSW: CMSSW_15_1_X_2025-03-20-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47642/45113/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 3 lines from the logs
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3929449
DQMHistoTests: Total failures: 101
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3929328
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 215 log files, 184 edm output root files, 50 DQM output files
TriggerResults: no differences found

- Obsolete parameters will cause a warning to be issued and then skipped in the validation. - The _cfi.py files will get a cms.obsolete parameter added which allows one to still set the parameter in python but will not be passed to the C++.

cmsbuild · 2025-03-20T20:15:00Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47642/44184

There are other open Pull requests which might conflict with changes you have proposed:
- File FWCore/ParameterSet/interface/ParameterDescription.h modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/interface/ParameterDescriptionNode.h modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterDescription.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterDescriptionBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/src/ParameterDescriptionNode.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216
- File FWCore/ParameterSet/src/ParameterSwitchBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/src/ParameterWildcardBase.cc modified in PR(s): Removed -Wconversion warnings in part of framework #47339
- File FWCore/ParameterSet/test/test_catch_ParameterSetDescription.cc modified in PR(s): Introduce vFloat and float types in ParameterSets #45216

cmsbuild · 2025-03-20T20:15:27Z

Pull request #47642 was updated. @Dr15Jones, @cmsbuild, @makortel, @smuzaffar can you please check and sign again.

Dr15Jones · 2025-03-20T20:15:41Z

NOTE: I made a quick attempt at removing the obsolete parameter if it appeared in the ParameterSet being validated. That didn't work in my attempt (the code threw an exception saying an unnamed parameter was present). I'd like to put such behavior off until a later PR to give more time to understand exactly the behavior.

Dr15Jones · 2025-03-20T20:15:48Z

please test

cmsbuild · 2025-03-21T00:12:46Z

+1

Size: This PR adds an extra 72KB to repository
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-83502d/45120/summary.html
COMMIT: 190f693
CMSSW: CMSSW_15_1_X_2025-03-20-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/47642/45120/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 1 lines from the logs
Reco comparison results: 6 differences found in the comparisons
DQMHistoTests: Total files compared: 50
DQMHistoTests: Total histograms compared: 3929449
DQMHistoTests: Total failures: 64
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3929365
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
Checked 215 log files, 184 edm output root files, 50 DQM output files
TriggerResults: no differences found

makortel · 2025-03-21T12:56:40Z

Comparison differences are related to #47071

makortel · 2025-03-24T13:58:37Z

+core

cmsbuild · 2025-03-24T13:58:57Z

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

mandrenguyen · 2025-03-25T19:20:11Z

+1

cmsbuild added this to the CMSSW_15_1_X milestone Mar 20, 2025

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 20, 2025

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 20, 2025

Dr15Jones force-pushed the obsoleteInPSetDescription branch from 53b9201 to 3b46cbe Compare March 20, 2025 15:50

cmsbuild added code-checks-pending tests-started and removed code-checks-rejected tests-pending labels Mar 20, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 20, 2025

makortel reviewed Mar 20, 2025

View reviewed changes

FWCore/ParameterSet/test/test_catch_ParameterSetDescription.cc Show resolved Hide resolved

makortel mentioned this pull request Mar 20, 2025

add support for "obsolete" parameters to the ParameterSet description and validation #13694

Closed

cmsbuild added tests-approved and removed tests-started labels Mar 20, 2025

Dr15Jones force-pushed the obsoleteInPSetDescription branch from 3b46cbe to 190f693 Compare March 20, 2025 20:13

cmsbuild removed tests-approved code-checks-approved labels Mar 20, 2025

cmsbuild added tests-pending code-checks-pending labels Mar 20, 2025

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 20, 2025

cmsbuild added tests-started and removed tests-pending labels Mar 20, 2025

cmsbuild added tests-approved and removed tests-started labels Mar 21, 2025

mmusich mentioned this pull request Mar 21, 2025

ConfigDB parsing/dumping using CMSSW_15_1_0_pre1 associated with a VPSet lead to incorrect python #47653

Open

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Mar 24, 2025

cmsbuild added orp-approved and removed orp-pending labels Mar 25, 2025

cmsbuild merged commit 5d817fc into cms-sw:master Mar 25, 2025
11 checks passed

Dr15Jones deleted the obsoleteInPSetDescription branch April 4, 2025 13:36

		ParameterDescriptionBase* addObsolete(U const& iLabel) {
		if constexpr (std::is_same_v<T, edm::ParameterSetDescription>) {

Conversation

Dr15Jones commented Mar 20, 2025

PR description:

PR validation:

Uh oh!

cmsbuild commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented Mar 20, 2025

Uh oh!

Dr15Jones commented Mar 20, 2025

Uh oh!

cmsbuild commented Mar 20, 2025

Uh oh!

cmsbuild commented Mar 20, 2025

Uh oh!

makortel Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cmsbuild commented Mar 20, 2025

Comparison Summary

Uh oh!

cmsbuild commented Mar 20, 2025

Uh oh!

cmsbuild commented Mar 20, 2025

Uh oh!

Dr15Jones commented Mar 20, 2025

Uh oh!

Dr15Jones commented Mar 20, 2025

Uh oh!

cmsbuild commented Mar 21, 2025

Comparison Summary

Uh oh!

makortel commented Mar 21, 2025

Uh oh!

makortel commented Mar 24, 2025

Uh oh!

cmsbuild commented Mar 24, 2025

Uh oh!

mandrenguyen commented Mar 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cmsbuild commented Mar 20, 2025 •

edited

Loading