Workaround for NanoAOD LHE weights in newer MadGraph #31680
Conversation
Fix a few places where missing_weightgroup wasn't set Debug back off Formatting
|
The code-checks are being triggered in jenkins. |
kdlong
changed the title
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-31680/18813
|
|
A new Pull Request was created by @kdlong (Kenneth Long) for master. It involves the following packages: PhysicsTools/NanoAOD @gouskos, @cmsbuild, @fgolf, @mariadalfonso, @santocch, @peruzzim can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
please test workflow 1325.6, 1325.7, 1325.8, 136.7952, 1329.1, 136.7722, 136.8521, 10826.0, 1325.61, 1325.81 |
|
The tests are being triggered in jenkins.
|
|
+1 |
|
Comparison job queued. |
|
Comparison is ready @slava77 comparisons for the following workflows were not done due to missing matrix map:
Comparison Summary:
|
|
@kdlong |
|
Hi @kdlong. Just a small comment. The blocks are currently scalewmg26x and scalewmg26xNew. Should we worry about more meaningful names or is it anyway unimportant once we have the restructured weights. Maybe we could at least add some info on what this is supposed to catch, or? |
|
@cms-sw/xpog-l2 @cms-sw/generators-l2 @cms-sw/analysis-l2 do you have any comments? |
|
Hi @silviodonato . My main concern is that, it becomes kind of hard to understand what each specific part is handling. It would be good to add a line for documentation to the code. Also, it is not clear to me if Dylan tested a file that has this problem or if this is just the code from Andrea. Sorry if I missed the info. |
|
@agrohsje the code is unquestionably a disaster and a nightmare to maintain. As you know it requires a major rewrite, which is a bit outside the scope. I agree that the names are silly, POWHEG even seems to fall into the mg26XNew format, but I didn't want to deal with trying to improve on this. It might be a good idea to do a little bit more testing if people are available and if there is any time before the v8 production starts. In particular I wonder if MadGraph Reweighting is still properly parsed in 2.6.5. |
|
Hi @kdlong . I fully agree the code is a nightmare to maintain but maybe we could still add a line with this is the problem, see e.g. this file. Moreover, as you say, it would be useful to test that it works well with a recent LO/NLO (madspin+ME reweighting) sample from 2.6.1 and 2.6.5. and there are no other outstanding issues. |
@kdlong can you confirm that you are taking care of this request? Regarding the request to add comments in the code to specify with part of the code handle which exceptions, I suggest to do on a separate note, together with the request to remove the std::cout #31939 (comment) |
|
I discussed a little more with @kdlong on Friday. The landscape is very rich. It is hard to make sure all problems are covered with the PR. I will approve for now, but for v9 we should definitely prepare a wider validation campaign. We need to collect samples with problems from MC contacts. We don't have such a list on GEN side or so. |
|
+1 |
|
+xpog testing shows no difference as expected for the MC that are not interested by this fix |
|
merge |
|
+1 |
cmsbuild
added
analysis-approved
fully-signed
and removed
analysis-pending
pending-signatures
labels
|
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 be automatically merged. |
PR description:
Workaround for a new header format in MadGraph >= 2.6.5. Originally by @arizzi, now updating to master.
PR validation:
Confirmed that 8 scale weights are found for MadGraph 2.6.5 sample:/ZGToLLG_01J_5f_PDFWeights_TuneCP5_13TeV-amcatnloFXFX-pythia8/RunIIAutumn18MiniAOD-102X_upgrade2018_realistic_v15-v2/MINIAODSIM
The central weight is not found, which is ok since it is always 1. Further testing is in progress.