-
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
GenEventInfoProduct weights size grows at lumisection change #27918
Comments
A new Issue was created by @peruzzim . @davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks. cms-bot commands are listed here |
assign generators |
New categories assigned: generators @alberto-sanchez,@qliphy,@efeyazgan,@agrohsje you have been requested to review this Pull request/Issue and eventually sign? Thanks |
From using
So the module which is creating the |
The weights for cmssw/GeneratorInterface/Pythia8Interface/plugins/Pythia8Hadronizer.cc Lines 678 to 733 in 982c595
|
The LuminosityBlocks->Scan("GenLumiInfoHeader_generator__GEN.obj.weightNames()") Then I get the results
So the 2nd LuminosityBlock in the job has more weights (matching what we see in GenEventInfoProduct) with the extra ones appearing to start at index 47 and being a repeat of all the |
I tested locally I could reproduce the issue with (1), i.e. increasing weights with LuminosityBlock While for (2) and (3), there is no such issue. For (3), the weight number is always 46, as below: root [1] LuminosityBlocks->Scan("GenLumiInfoHeader_generator__SIM.obj.weightNames()")
|
Tested further with EGM-RunIIWinter19PFCalib17wmLHEGS-00001, and found the issue appears already with 10_5_X. Note since 10_5_X, we have Pythia8 2.40 I also notice "fMasterGen->info.nWeights()" (a Pythia8 function to access uncertainty weights) is increasing with LuminosityBlock Adding @mkirsano @alberto-sanchez @intrepid42 who should be able to check more above. |
Adding @smrenna It seems to me possibly due to Pythia8 itself. As mentioned above, the issue appears since 10_5_X, i.e. from Pythia8.240 There are some differences on PS weights definitions between Pythia8.240 and 8.230: https://github.com/cms-externals/pythia8/blob/cms/240/src/SimpleSpaceShower.cc#L3136 https://github.com/cms-externals/pythia8/blob/cms/230/src/SpaceShower.cc#L3040 where the space shower (isr) part is different. For Pythia8.240, nWeights possibly gets increased at each initialization, that may explain why we see duplications from isr as mentioned above: Note according to PS Weight fragment, there are totally 22 variations from isr, this may also explain the number 22 mentioned above by Marco: So possibly we should change in I will leave this for the Pythia author, @smrenna to comment here. |
Dear @qliphy, |
But as shown here: so it seems after each initialization, nWeights can get increased. |
@srimanob |
I modified an example file in the Pythia8 distribution, and I confirm that the number of weights change upon a second initialization. However, I am not 100 percent sure this is what CMSSW does. What I don’t know is this: what does CMSSW expect to happen? |
Please make the following change to Pythia.cc: // Initialize the random number generator. // Find which frame type to use. |
Did a pull request in the cms externals branch: |
Thanks @smrenna. I tested the pull request and it is working fine. To summarize what I did. |
@agrohsje so your check seems to confirm on a few events that the fix is doing what expected without any apparent drawback. At this point I would suggest to integrate this version into a build to perform more extensive checks |
@fabiocos Yes it would be very nice to have a build and release with this fix included, as the other workaround (e.g. filtering unwanted weights) may still take time. Thanks! |
Can we consider that this issue is closed with the fix in cms-sw/cmsdist#5229 (backport to 10_6 cms-sw/cmsdist#5239) ? Thanks you all. |
If the GEN group has a MINIAOD test file generated with the new Pythia version, I can test NANOAOD on it, but this is something for them to sign off in the end |
Let me quote below what Alexander has checked in another email thread. Everything looks ok there. Dear all, |
Can we consider this item closed? If so GEN please sign |
+1 |
This issue is fully signed and ready to be closed. |
On request of PPD and release managers, I'm reporting here to follow up on https://its.cern.ch/jira/browse/CMSCOMPPR-8777.
It has been found that the size of the vector returned by the
weights()
method of GenEventInfoProduct is not constant within a MINIAOD file in the UL2017 MC production (in CMSSW_10_6_2). This was exposed by the fact that NANOAOD production code reads this object to store PS weights, as it is normally done in analysis.Later checks showed that the issue appears already in GENSIM files (e.g. in
/store/mc/RunIISummer19UL17SIM/GluGluToHiggs0Mf05ph0continToZZTo2e2tau_M125_10GaSM_13TeV_MCFM701_pythia8/SIM/106X_mc2017_realistic_v6-v2/260000/FE9FEFBF-1735-784F-B84C-338F8A41624C.root
).It was observed that the size of the vector often grows by 22 at a lumisection change, potentially up to very large numbers (~2000) - but sometimes it goes back to the expected value of 46, probably as a result of merging separate output files from an earlier production step.
On the example file above, it seems that
1.0
values somehow appear in the central part of the vector, increasing its size. The rest of the values are all close to 1 and do not show crazy values.It was suggested that this issue could somehow be related to #25708.
@srimanob @fabiocos
The text was updated successfully, but these errors were encountered: