-
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
Tighten check for the number of weights in GenWeightValidation #37185
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37185/28760
|
A new Pull Request was created by @perrotta (Andrea Perrotta) for master. It involves the following packages:
@SiewYan, @mkirsano, @emanueleusai, @ahmad3213, @cmsbuild, @GurpreetSinghChahal, @jfernan2, @Saptaparna, @alberto-sanchez, @pmandrik, @pbo0, @rvenditti can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
Thank you very much @perrotta, it looks good to me |
Thank you to you @SanghyunKo : it was your suggestion in fact! |
type bugfix |
urgent |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-daeffd/22982/summary.html Comparison SummarySummary:
|
Plots now get null entries while before they had entries |
+1 |
+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 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) |
+1 |
Random differences in the generator weights have disappeared now, after the merging in the IB of this PR, see for example the results of the tests in 37161 |
PR description:
Following a couple of issues reported after the merging of #36994:
GenWeightValidation::analyze
, as reported by ASAN, see Add GEN/LHE weight validation subdirectories for GEN Relval #36994 (comment)@SanghyunKo suggested in #36994 (comment) a quite reasonable fix for them.
To speed up the integration in view of the 12_3_0pre6 deadline, I implement here the fix of @SanghyunKo, while waiting for some possible further feedback
Of course, if @SanghyunKo or anyone else finds a more appropriate solution, this PR can be closed and we can move to the new one.
PR validation: