-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Move default Run 4 workflows to D121 #49437
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
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49437/46917
|
|
A new Pull Request was created by @kpedro88 for master. It involves the following packages:
@AdrianoDee, @Alejandro1400, @DickyChant, @JanChyczynski, @antoniovagnerini, @arunhep, @atpathak, @cmsbuild, @davidlange6, @fabiocos, @francescobrivio, @ftenchini, @mandrenguyen, @miquork, @perrotta can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
Open questions:
|
| defaultDataSets['Run4D96']='CMSSW_13_1_0_pre1-130X_mcRun4_realistic_v2_2026D96noPU-v' | ||
| defaultDataSets['Run4D98']='CMSSW_13_2_0_pre1-131X_mcRun4_realistic_v5_2026D98noPU-v' | ||
| defaultDataSets['Run4D110']='CMSSW_15_1_0_pre5-150X_mcRun4_realistic_v1_STD_RegeneratedGS_Run4D110_noPU-v' | ||
| defaultDataSets['Run4D121']='' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cms-sw/pdmv-l2 is there anything that we can put here? I could not find any D121 samples in DAS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we ever produce relvals with D121?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really. We can produce them.
In general I think that if we want to move definetly to D121, we'd need a validation campaign.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should hopefully be very straightforward to validate, because the changes for all subdetectors are quite minor. We expect larger changes in 2026, but we wanted to complete this move first (which started in the summer).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe @AdrianoDee could help to submit minbias relvals with D121 with existing release (e.g. 16_0_0_pre1 or pre2). Then, within the next few days, when dataset is available, this PR can be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, not sure how short matrix for PR test will work for now, as it will try to run D121+PU workflow, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including pre2 relvals for this geometry in this PR would be ideal.
Indeed, a few tests may fail, but I think most do not include PU.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've sent a set of D121 relvals here. Most of them are already done. So there you can use CMSSW_16_0_0_pre2-150X_mcRun4_realistic_v1_STD_RegeneratedGS_Run4D121_noPU-v.
For D121 validation itself, since it should be minor, I would proceed by just having both D110 and D121 samples produced for the incoming 16_0_0_pre3 campaign and have the validators inspect them there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @AdrianoDee ! Your proposal for validation sounds good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I shall update the ones in Geometry package
|
please test |
|
-1 Failed Tests: RelVals Failed RelVals |
|
test parameters:
|
this is going to be discussed this week at the upgrade HLT meeting. |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-49437/46988
|
|
Pull request #49437 was updated. @AdrianoDee, @Alejandro1400, @DickyChant, @JanChyczynski, @antoniovagnerini, @arunhep, @atpathak, @cmsbuild, @davidlange6, @fabiocos, @francescobrivio, @ftenchini, @mandrenguyen, @miquork, @perrotta can you please check and sign again. |
|
please test |
|
+1 Size: This PR adds an extra 20KB to repository DAS Queries: The DAS query tests failed, see the summary page for details. Comparison SummarySummary:
|
|
+db |
|
+pdmv |
|
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. @ftenchini, @sextonkennedy, @mandrenguyen (and backports should be raised in the release meeting by the corresponding L2) |
|
+1 |
PR description:
This PR is intended to complete the update of the default Run 4 geometry to D121, started in #48297.
PR validation:
runTheMatrix.py lists the correct workflows.
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:
Not a backport. Can be backported upon request, but no immediate needs are foreseen.