-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set concurrent lumis/IOVs in ConfigBuilder if their value is different from default regardless of the number of threads #37417
Conversation
…gardless of the number of threads It can happen that the job configuration is generated with cmsDriver without specifying the number of threads (i.e. that is 1), and ends up being used with number of threads being overridden. If the number of concurrent lumis/IOVs is set different from the default in such case, that setting would be lost.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37417/29093
|
A new Pull Request was created by @makortel (Matti Kortelainen) for master. It involves the following packages:
@cmsbuild, @perrotta, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@cmsbuild, please test workflow 5.2,140.0,521.0,7.0,300.0,140.0,5.5,511.0,281.0,8.1,534.0,281.0,132.0,280.0,120.0 |
ping bot |
let me see why bot did not run the cmparison job |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8b7d03/23547/summary.html Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
This parameter was set to False already in cms-sw#34366.
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37417/29131
|
@cmsbuild, please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8b7d03/23611/summary.html Comparison SummarySummary:
|
+1 |
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 be automatically merged. |
@jordan-martins @makortel A backport PR to 12_2_x/12_3_x is needed. |
PR description:
As discussed in Mattermost https://mattermost.web.cern.ch/cms-o-and-c/pl/sw55fqpr7pna8x581q453wcsne and reported in #37385, it can happen that the job configuration is generated with cmsDriver without specifying the number of threads (i.e. that is 1), and ends up being used with number of threads being overridden. If the number of concurrent lumis/IOVs is set different from the default in such case, that setting would be lost.
This PR overrides the
process.options.numberOfConcurrentLuminosityBlocks
andprocess.options.eventSetup.numberOfConcurrentIOVs
when they have been changed from their defaults, regardless of the number of threads.PR validation:
Checked with
limited,5.2,140.0,521.0,7.0,300.0,140.0,5.5,511.0,281.0,8.1,534.0,281.0,132.0,280.0,120.0
withrunTheMatrix.py
both-t 2
option and without that the special cases that "need" to be explicitly set to use 1 concurrent lumi get that override in both cases. (I used the same set of workflows as in #35302)