Add ESInputTag to VPSet default added to cfi file #44344

Dr15Jones · 2024-03-07T16:19:10Z

PR description:

Also added unit test for cfi content of VPSet.

PR validation:

Code compiles and new unit test passes.

cmsbuild · 2024-03-07T16:19:38Z

cms-bot internal usage

cmsbuild · 2024-03-07T16:24:23Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44344/39377

This PR adds an extra 20KB to repository

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-44344/39377/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44344/39377/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

Dr15Jones · 2024-03-07T16:30:27Z

please test

cmsbuild · 2024-03-07T16:34:44Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44344/39378

This PR adds an extra 24KB to repository

cmsbuild · 2024-03-07T16:35:06Z

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

It involves the following packages:

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.
@antoniovilela, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

Dr15Jones · 2024-03-07T16:56:59Z

FWCore/ParameterSet/src/FillDescriptionFromPSet.cc

+    s_findTheRightFunction[edm::k_ESInputTag] = &fillDescriptionFromParameter<edm::ESInputTag>;
+    s_findTheRightFunction[edm::k_VESInputTag] = &fillDescriptionFromParameter<std::vector<edm::ESInputTag>>;


These were the two missing entries.

The rest should just be a direct replacement of the char with the same valued enum.

makortel · 2024-03-07T17:00:13Z

FWCore/ParameterSet/test/test_catch_ParameterSetDescription.cc

+    vl = cms.vint64(1),
+    vt = cms.VInputTag('foo'),
+    vui = cms.vuint32(1),
+    vul = cms.vuint64(1)


I see string, vstring, FileInPath, vbool are "missing" from this list. Are they tested well-enough elsewhere, or would there be value in adding them here?

(in principle also inner PSet and VPSet are not there, but maybe they are tested already well-enough elsewhere?)

We don't have a vbool in the system. Seems FileInPath's can't be created without the file being there (there is a special test in this unit test working around that). I'll definitely add the string and vstring.

We have

cmssw/FWCore/ParameterSet/python/Types.py

Line 1049 in 5b9428b

class vbool(_ValidatingParameterListBase):

It is not listed in https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideAboutPythonConfigFile#Parameters though

I was able to add all the presently available options to the test.

cmsbuild · 2024-03-07T19:20:00Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3a6a0d/37961/summary.html
COMMIT: c8b0d1a
CMSSW: CMSSW_14_1_X_2024-03-06-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44344/37961/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 7 lines to the logs
Reco comparison results: 56 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3293428
DQMHistoTests: Total failures: 1355
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3292053
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

Also added unit test for cfi content of VPSet.

cmsbuild · 2024-03-07T19:43:41Z

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

cmsbuild · 2024-03-07T22:33:04Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-3a6a0d/37970/summary.html
COMMIT: 2506dd5
CMSSW: CMSSW_14_1_X_2024-03-06-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44344/37970/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially removed 85 lines from the logs
Reco comparison results: 48 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3293428
DQMHistoTests: Total failures: 6
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3293402
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

makortel · 2024-03-08T14:10:33Z

The usual

makortel · 2024-03-08T14:10:36Z

+core

cmsbuild · 2024-03-08T14:10:58Z

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

antoniovilela · 2024-03-09T14:15:23Z

+1

Martin-Grunewald · 2024-03-10T07:33:41Z

In the most recent relvals (CMSSW_14_1_X_2024-03-10-0000), there are many errors of the type:

Traceback (most recent call last):
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/bin/el8_amd64_gcc12/cmsDriver.py", line 40, in <module>
    run()
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/bin/el8_amd64_gcc12/cmsDriver.py", line 16, in run
    configBuilder.prepare()
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/Configuration/Applications/python/ConfigBuilder.py", line 2284, in prepare
    self.loadAndRemember(self.EVTCONTDefaultCFF)  #load the event contents regardless
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/Configuration/Applications/python/ConfigBuilder.py", line 354, in loadAndRemember
    self.process.load(includeFile)
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/FWCore/ParameterSet/python/Config.py", line 761, in load
    module = __import__(moduleName)
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/Configuration/EventContent/python/EventContent_cff.py", line 64, in <module>
    from RecoHGCal.Configuration.RecoHGCal_EventContent_cff import *
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/RecoHGCal/Configuration/python/RecoHGCal_EventContent_cff.py", line 3, in <module>
    from RecoHGCal.TICL.iterativeTICL_cff import ticlIterLabelsMerge
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/RecoHGCal/TICL/python/iterativeTICL_cff.py", line 16, in <module>
    from RecoHGCal.TICL.tracksterSelectionTf_cfi import *
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/src/RecoHGCal/TICL/python/tracksterSelectionTf_cfi.py", line 1, in <module>
    from PhysicsTools.TensorFlow.tfGraphDefProducer_cfi import tfGraphDefProducer as _tfGraphDefProducer
  File "/cvmfs/cms-ib.cern.ch/sw/x86_64/week0/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_X_2024-03-10-0000/cfipython/el8_amd64_gcc12/PhysicsTools/TensorFlow/tfGraphDefProducer_cfi.py", line 5, in <module>
    FileName = cms.FileInPath(),
TypeError: __init__() missing 1 required positional argument: 'value'

It seems they originate from this PR which touches FileInPath though I do not understand why the PR tests of this PR went fine.
I suppose for fillDescriptions and cfi usage, both cms.FileInPath('') and cms.FileInPath() should be valid.

This PR changes:

cms.FileInPath('')

to

cms.FileInPath()

in fillDescriptions-generated cfi files like the above.

Dr15Jones · 2024-03-10T15:40:05Z

@Martin-Grunewald thanks for reporting the problem. I'll work on a fix today.

Martin-Grunewald · 2024-03-10T20:01:57Z

Fixed by #44362

antoniovilela · 2024-03-10T21:49:00Z

Fixed by #44362

this is now merged

antoniovilela · 2024-03-10T21:51:32Z

It seems they originate from this PR which touches FileInPath though I do not understand why the PR tests of this PR went fine.

It would be nice to understand why.

Dr15Jones · 2024-03-10T22:35:41Z

It would be nice to understand why

This PR, I changed the 'empty' generated cfi version of FileInPath from cms.FileInPath('') to cms.FileInPath() which turned out not to parse properly in python.

antoniovilela · 2024-03-11T12:19:11Z

It would be nice to understand why

This PR, I changed the 'empty' generated cfi version of FileInPath from cms.FileInPath('') to cms.FileInPath() which turned out not to parse properly in python.

Thanks. But why nothing showed up in PR tests?

Dr15Jones · 2024-03-11T13:36:17Z

Thanks. But why nothing showed up in PR tests?

That I do not know but would like to find out.

makortel · 2024-03-11T13:40:24Z

This PR touched only source files in FWCore/ParameterSet and FWCore/Utilities, and therefore no other packages (except Utilities/RelMon and Utilities/StaticAnalyzer, don't know why those were) were checked out in the PR tests, and therefore none of the tests that failed in IBs were run in the PR tests.

In retrospect we maybe should have tested this PR with full_cmssw = true.

cmsbuild added this to the CMSSW_14_1_X milestone Mar 7, 2024

cmsbuild added core-pending pending-signatures tests-pending orp-pending code-checks-pending labels Mar 7, 2024

cmsbuild added code-checks-rejected and removed code-checks-pending labels Mar 7, 2024

Dr15Jones force-pushed the fixESInputTagCfi branch from bf22c54 to c8b0d1a Compare March 7, 2024 16:28

cmsbuild added code-checks-pending and removed code-checks-rejected labels Mar 7, 2024

cmsbuild added tests-started and removed tests-pending labels Mar 7, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Mar 7, 2024

Dr15Jones commented Mar 7, 2024

View reviewed changes

makortel reviewed Mar 7, 2024

View reviewed changes

cmsbuild added tests-approved and removed tests-started labels Mar 7, 2024

Dr15Jones added 2 commits March 7, 2024 13:25

Add ESInputTag to VPSet default added to cfi file

417bbb6

Also added unit test for cfi content of VPSet.

Allow storage of an empty FileInPath

397ec1e

Dr15Jones force-pushed the fixESInputTagCfi branch from c8b0d1a to f3810bb Compare March 7, 2024 19:29

cmsbuild removed tests-approved code-checks-approved labels Mar 7, 2024

makortel mentioned this pull request Mar 7, 2024

Add ESInputTag to VPSet default added to cfi file cms-sw/framework-team#846

Closed

cmsbuild added tests-approved and removed tests-started labels Mar 7, 2024

cmsbuild added core-approved fully-signed and removed core-pending pending-signatures labels Mar 8, 2024

makortel mentioned this pull request Mar 8, 2024

Auto-generated cfi file missing an ESInputTag parameter in a VPSet #44307

Closed

cmsbuild added orp-approved and removed orp-pending labels Mar 9, 2024

cmsbuild merged commit 9a93dd7 into cms-sw:master Mar 9, 2024
11 checks passed

This was referenced Mar 10, 2024

Update to CUDA 12.4.0 cms-sw/cmsdist#9046

Merged

Updated root to tip of branch master cms-sw/cmsdist#9056

Merged

Dr15Jones deleted the fixESInputTagCfi branch March 13, 2024 16:14

Dr15Jones mentioned this pull request Mar 19, 2024

Add ESInputTag to VPSet default added to cfi file [14_0] #44465

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ESInputTag to VPSet default added to cfi file #44344

Add ESInputTag to VPSet default added to cfi file #44344

Dr15Jones commented Mar 7, 2024

cmsbuild commented Mar 7, 2024 •

edited

Loading

cmsbuild commented Mar 7, 2024

Dr15Jones commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

Dr15Jones Mar 7, 2024

makortel Mar 7, 2024

Dr15Jones Mar 7, 2024

makortel Mar 7, 2024

Dr15Jones Mar 7, 2024

cmsbuild commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

makortel commented Mar 8, 2024

makortel commented Mar 8, 2024

cmsbuild commented Mar 8, 2024

antoniovilela commented Mar 9, 2024

Martin-Grunewald commented Mar 10, 2024 •

edited

Loading

Dr15Jones commented Mar 10, 2024

Martin-Grunewald commented Mar 10, 2024

antoniovilela commented Mar 10, 2024

antoniovilela commented Mar 10, 2024

Dr15Jones commented Mar 10, 2024

antoniovilela commented Mar 11, 2024

Dr15Jones commented Mar 11, 2024

makortel commented Mar 11, 2024 •

edited

Loading

		s_findTheRightFunction[edm::k_ESInputTag] = &fillDescriptionFromParameter<edm::ESInputTag>;
		s_findTheRightFunction[edm::k_VESInputTag] = &fillDescriptionFromParameter<std::vector<edm::ESInputTag>>;

Add ESInputTag to VPSet default added to cfi file #44344

Add ESInputTag to VPSet default added to cfi file #44344

Conversation

Dr15Jones commented Mar 7, 2024

PR description:

PR validation:

cmsbuild commented Mar 7, 2024 • edited Loading

cmsbuild commented Mar 7, 2024

Dr15Jones commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

Dr15Jones Mar 7, 2024

Choose a reason for hiding this comment

makortel Mar 7, 2024

Choose a reason for hiding this comment

Dr15Jones Mar 7, 2024

Choose a reason for hiding this comment

makortel Mar 7, 2024

Choose a reason for hiding this comment

Dr15Jones Mar 7, 2024

Choose a reason for hiding this comment

cmsbuild commented Mar 7, 2024

Comparison Summary

cmsbuild commented Mar 7, 2024

cmsbuild commented Mar 7, 2024

Comparison Summary

makortel commented Mar 8, 2024

makortel commented Mar 8, 2024

cmsbuild commented Mar 8, 2024

antoniovilela commented Mar 9, 2024

Martin-Grunewald commented Mar 10, 2024 • edited Loading

Dr15Jones commented Mar 10, 2024

Martin-Grunewald commented Mar 10, 2024

antoniovilela commented Mar 10, 2024

antoniovilela commented Mar 10, 2024

Dr15Jones commented Mar 10, 2024

antoniovilela commented Mar 11, 2024

Dr15Jones commented Mar 11, 2024

makortel commented Mar 11, 2024 • edited Loading

cmsbuild commented Mar 7, 2024 •

edited

Loading

Martin-Grunewald commented Mar 10, 2024 •

edited

Loading

makortel commented Mar 11, 2024 •

edited

Loading