-
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
Herwig lhe matching fix #40939
Herwig lhe matching fix #40939
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40939/34425
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40939/34426
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-40939/34427
|
A new Pull Request was created by @Dominic-Stafford for master. It involves the following packages:
@SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @alberto-sanchez, @menglu21, @GurpreetSinghChahal can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test with cms-sw/cmsdist#8349 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7862e3/31026/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
please test 537 and 538 |
please test 537 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7862e3/31632/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummarySummary:
|
+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, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
Thanks, Andrea! |
please test workflow 537.0, 538.0 |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-7862e3/31646/summary.html The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:
You can see more details here: Comparison SummaryThere are some workflows for which there are errors in the baseline: Summary:
|
Clearly, plots in the DQM/Generator folder are not reproducible, at least in wf 538. For example, the first plot that I showed above now reads This is something @cms-sw/generators-l2 should take care of. On the other hand, these non reproducibilities does not seem to depend on this PR: let have it merged, then, and the group can continue investigating on their origin. Wf 537 does not seem affected (only changes related to this PR are visible in the DQM comparisons for that workflow), something that could shed some light on the origin of the non reproducibility. |
+1 |
@Dominic-Stafford since the merging of this PR IBs are crashing with the following error message (e.g. from wf 512.0):
Please notice that the very same error appears independently on the merge of cms-sw/cmsdist#8409, which I forgot for CMSSW_13_1_X_2023-03-29-1100 and merged later for CMSSW_13_1_X_2023-03-29-2300, but as you can see both IBs are crashing with the same error message. Could you please have a look and provide a fix at your earliest? |
PR description:
Adds a HadronizerFilter mode to the Herwig7Interface, which uses numbering of LHE events to ensure that the LHE and Gen level events in the CMS event record match up- previously this was not the case for processes with merging, as Herwig would skip events silently. Should be tested with cms-sw/cmsdist#8349, which propagates the LHE numbering through Herwig.
PR validation:
Have tested the functionality works in CMSSW_10_6. Have also updated all Herwig validation examples with the new HadronizerFilter, and they all work