-
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
Nano test in runthematrix #39472
Nano test in runthematrix #39472
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39472/32202
|
A new Pull Request was created by @vlimant (vlimant) for master. It involves the following packages:
@bbilin, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @sunilUIET can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@@ -87,7 +89,8 @@ def reset(self, what='all'): | |||
'relval_2026':True, | |||
'relval_identity':False, | |||
'relval_machine':True, | |||
'relval_premix':True | |||
'relval_premix':True, | |||
'relval_nano':False |
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.
@vlimant , I would suggest to enable it for default runTheMatrix . This will allow to run these workflows for IBs. This way we make sure that relvals are working and automatically cache the input used by these wfs.
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 is partially heavy to run all these, are you sure @smuzaffar that we want to enable them by default ?
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.
@vlimant , for IBs, yes we should enable these all to make sure that they all work.
For PR tests, we can select a few of these for which bot should run the comparisons.
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.
in 9806eb4
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39472/32219
|
Pull request #39472 was updated. @bbilin, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @sunilUIET can you please check and sign again. |
to be noted : 91a8801 |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-39472/32220
|
Pull request #39472 was updated. @bbilin, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @sunilUIET can you please check and sign again. |
test parameters:
|
please test |
-1 Failed Tests: RelVals RelVals
|
thanks @kskovpen and apologies, please re-sign |
+pdmv |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-1debec/28156/summary.html Comparison Summary@slava77 comparisons for the following workflows were not done due to missing matrix map:
Summary:
|
@cms-sw/upgrade-l2 please sign or comment further. |
+Upgrade |
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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
@vlimant scrolling through the thread I understand that this is not a Work In Progress any more, and it can be merged in master. If so, could you please update the title and remove "[WIP]"? (Or I can do if you confirm so) |
I edited the title accordingly, thanks |
+1 |
PR description:
as wished in #39452 the first step is to integrate some workflows in runTheMatrix. Here is a first stab at this, knowing that it's missing a couple of workflows, and that some will go away soon too.
PR validation:
runTheMatrix.py --sites '' --das-options '--limit 5' --what nano
did not run to full completion because some input datasets are deprecated, and some are not present on disk anymore.
EDIT:
post-facto, the behaviour of the MatrixReader had to be slightly changed to take properly into account run:lmi ranges. Thus far only fist three were considered and inconsistently if more than 3 were provided.
we found out existing nano workflows 136.7722, 136.7952, 136.8521, 136.8522, 136.8523, 1325.6, 1325.7, 1325.8, 1325.61, 1325.81, 1329.1 that are superseded by those put in place by this PR. we removed them and swapped 1325.81 for 2500.601