-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add D88 wfs to the short matrix #36833
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36833/28011
|
A new Pull Request was created by @srimanob (Phat Srimanobhas) for master. It involves the following packages:
@jordan-martins, @bbilin, @wajidalikhan, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
-1 Failed Tests: RelVals RelVals-INPUT RelValsRelVals-INPUT
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36833/28016
|
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-da3bae/22113/summary.html Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
+Upgrade The PR introduce D88 workflows to the short matrix. We will have another PR to remove D77/D86 when the relvals migration is complete. PR test run fine, no change is expected. I think we should start running D88 in a short matrix with PR test for a while before we cut the release and produce relvals. Note to @cms-sw/reconstruction-l2 to prepare for comparison config. |
@cms-sw/pdmv-l2 This PR is good to review. The premixing wf will be added later, together with cleanup when we complete the migration. |
+pdmv Looks good to us |
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, @qliphy (and backports should be raised in the release meeting by the corresponding L2) |
@@ -3464,6 +3464,7 @@ def gen2021HiMix(fragment,howMuch): | |||
defaultDataSets['2026D49']='CMSSW_12_0_0_pre4-113X_mcRun4_realistic_v7_2026D49noPU-v' | |||
defaultDataSets['2026D76']='CMSSW_12_0_0_pre4-113X_mcRun4_realistic_v7_2026D76noPU-v' | |||
defaultDataSets['2026D77']='CMSSW_12_1_0_pre2-113X_mcRun4_realistic_v7_2026D77noPU-v' | |||
#defaultDataSets['2026D88']='CMSSW_12_2_0_pre3-122X_mcRun4_realistic_v4_2026D88noPU-v1' |
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.
Why is this line commented out?
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 first wanted to add also premix workflow to the short matrix test as other Phase-2 defaults. However, I think it is better to include it when we have updated MinBias from the first relvals in 12_3_0_pre5. So, this line will be updated and uncomment.
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.
If you need a clean PR, I can remove it for now.
+1 |
FYI @cms-sw/reconstruction-l2 We include 3 workflows to the short matrix, including
|
@srimanob @cms-sw/pdmv-l2 There appear several wf failures in IB after this PR gets merged. the query is file dataset = /RelValMinBias_14TeV/1/GEN-SIM Maybe we should update the mixing dataset? |
@qliphy |
@srimanob yes, anything in relvals_2026 can run in IBs. |
PR description:
As foreseen on move relvals to D88 in 12_3_0_pre5, this PR adds D88 workflows to the short matrix. We then can remove D86 and D77 in the later PR after migration to D88 is complete.
In addition, two workflows (39496.0,39500.0) of CloseByPGun are added to the short matrix as discussed in #36832
PR validation:
if this PR is a backport please specify the original PR and why you need to backport that PR:
Not a backport, and no need of backport