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
Bug fix. Add 2 overrides to LHESource #20883
Conversation
Add readRunAuxiliary_ and readLuminosityBlockAuxiliary_ function overrides to LHESource. (also includes code-check fixes and indentation fixes that remove inconsistencies and tabs, but the only substantive changes are the 2 new functions). Earlier changes in the Framework broke LHESource and the addition of these two functions fixes the problem by setting the ProcessHistoryID properly.
The code-checks are being triggered in jenkins. |
+code-checks |
please test |
The tests are being triggered in jenkins. |
A new Pull Request was created by @wddgit (W. David Dagenhart) for master. It involves the following packages: GeneratorInterface/LHEInterface @cmsbuild, @efeyazgan, @perrozzi, @thuer, @govoni can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
assign core |
This fixes a problem reported in the hypernews. See: I suspect there are no tests of LHESource in the unit tests or runTheMatrix, because it looks like the problem fixed here would probably have made LHESource always fail in 9_3_X and 9_4_X. Note that the only test I've done is running and verifying that with this fix the process in the hypernews report runs successfully to completion without an exception. If there is an expert for LHESource (I'm not), they should probably run some checks on this. I will backport the fix to 9_3_X. |
New categories assigned: core @Dr15Jones,@smuzaffar you have been requested to review this Pull request/Issue and eventually sign? Thanks |
+1 |
-1 Tested at: 45a584d The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/1000.0_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT/step3_RunMinBias2011A+RunMinBias2011A+TIER0+SKIMD+HARVESTDfst2+ALCASPLIT.log The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
The RelVal failure of 1000.0 is unrelated to this PR. The same failure is occurring in the IBs. |
please test |
The tests are being triggered in jenkins. |
+1 The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic: |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
+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. @davidlange6, @slava77, @smuzaffar (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Add readRunAuxiliary_ and readLuminosityBlockAuxiliary_
function overrides to LHESource. (also includes code-check
fixes and indentation fixes that remove inconsistencies and
tabs, but the only substantive changes are the 2 new functions).
Earlier changes in the Framework broke LHESource and the
addition of these two functions fixes the problem by setting
the ProcessHistoryID properly.