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
Fix to save PS weights from Pythia8 in GenEventInfoProduct record #21073
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/PR-21073/1705 Code check has found code style and quality issues which could be resolved by applying a patch in https://cmssdt.cern.ch/SDT/code-checks/PR-21073/1705/git-diff.patch You can run |
The code-checks are being triggered in jenkins. |
+code-checks |
A new Pull Request was created by @jfernan2 for master. It involves the following packages: GeneratorInterface/Pythia8Interface @cmsbuild, @efeyazgan, @perrozzi, @thuer, @govoni can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
@jfernan2 can you please propagate this to 93X too? |
…n using Powheg. Backport of cms-sw#21073
-1 Tested at: 5e17d0f 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/10042.0_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017/step5_ZMM_13+ZMM_13TeV_TuneCUETP8M1_2017_GenSimFull+DigiFull_2017+RecoFull_2017+ALCAFull_2017+HARVESTFull_2017.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 error in relval is related to DQM histograms, not by the code change itself, I would suggest retesting... |
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) |
@@ -807,6 +807,15 @@ bool Pythia8Hadronizer::hadronize() | |||
nFSRveto += fEmissionVetoHook->getNFSRveto(); | |||
} | |||
|
|||
// fill shower weights | |||
// http://home.thep.lu.se/~torbjorn/pythia82html/Variations.html | |||
if( fMasterGen->info.nWeights() > 1 ){ |
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.
hi @jfernan2 - why protect against >1 here? It seems this will mean users all have to protect against this case downstream.
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.
This is the way it has been done in other parts of the code in L753 and L959. Weights are only saved if their size is >0, that's my understanding. What are you proposing instead? Thanks
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.
thanks I had missed that (not that I understand it, but consistency is good)
+1 |
Fix to save PS weights from Pythia8 (Backport of #21073)
When using Powheg, it seems this piece of code got lost at some point. Not sure if this is the optimal solution but it is done this way in other parts of the class and works for me.
I let the experts to comment, perhaps a backport to 93X is needed if we want to use it for Moriond18 production.