-
Notifications
You must be signed in to change notification settings - Fork 176
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
[10_6] Backport Pythia fix for alphaS running #8555
Conversation
Activate backported fixes for alphaS running
A new Pull Request was created by @mseidel42 (Markus Seidel) for branch IB/CMSSW_10_6_X/gcc700. @cmsbuild, @smuzaffar, @aandvalenzuela, @iarspider can you please review it and eventually sign? Thanks. |
hold
|
Pull request has been put on hold by @rappoccio |
Hi, This feature had a validation to assess the impact, right? If so, can the studies be shared? In any case, I would propose some official validation to be delivered by PdmV based on GEN-related people (please open a JIRA ticket in the PDMVRELVALS project). Thanks, |
Hi Jordan, The impact on some ttbar observables was shown in the GEN meeting on Monday: https://indico.cern.ch/event/1293969/contributions/5438463/attachments/2668515/4624955/GEN%20Meeting%20190623%20v2.pdf#page=16 We think that this fix is the crucial change between Pythia 8.307 and 8.309 shown there. However, we agreed to leave the defaults at the old (buggy) behavior and added new Pythia options to activate the bugfix for special samples requested by experts. Best, |
@mseidel42 i think @jordan-martins / ppd just wants to doubly make sure that pythia option switches you added functions as it is designed, just in case to avoid surprises. gen might comment more |
Hi @jordan-martins, I think @mseidel42 already explained well the motivation of this backport, and the validation using ttbar in https://indico.cern.ch/event/1293969/contributions/5438463/attachments/2668515/4624955/GEN%20Meeting%20190623%20v2.pdf#page=16 shows expected results from the fix. Since some UL requests i.e., bb4l may need this new feature (the correct results rather than the coincident-"correct" result) and I think it's worth to have it in the 10_6, or you want some more validation on other processes |
we did such campaigns in the past via an extended relval. wouldn't;t that make sense to make sure we didn't miss anything. not that I don't trust markus. just to be 100% certain. |
please test |
-1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9d279/33546/summary.html External BuildI found compilation warning when building: See details on the summary page. |
@cms-sw/generators-l2 , we need cms-externals/pythia8#30 to be signed/megred first and then we can start testing this PR (as this PR will need changes e.g. correct commit of cms-externals/pythia8 after the merge of cms-externals/pythia8#30) |
please test |
Pull request #8555 was updated. |
@rappoccio , should we unhold this PR now? |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f9d279/34897/summary.html Comparison SummarySummary:
|
+externals @rappoccio @antoniovilela , feel free to unhold and merge it for next 10.6.X Ibs/releases |
Just noticed that this is still open |
@rappoccio , could we unhold this PR |
@rappoccio a kind ping |
unhold |
+1 |
This pull request is fully signed and it will be integrated in one of the next IB/CMSSW_10_6_X/gcc700 IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @rappoccio, @sextonkennedy, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
merge |
Activate backported fixes for alphaS running cms-externals/pythia8#30