Fix cross section when using ResonanceDecayFilter #46725
Conversation
|
cms-bot internal usage |
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/42697 |
|
A new Pull Request was created by @manuel-gonzalez-henandez for master. It involves the following packages:
@bbilin, @cmsbuild, @lviliani, @menglu21, @mkirsano can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
|
@Dominic-Stafford please have a look. |
|
Hi, the code looks good to me, and none of the tests that failed due to missing files look like they're related to Pythia (which should be workflows 503-510, 512-528, 552-556 and 560-562) |
|
Milestone for this pull request has been moved to CMSSW_15_0_X. Please open a backport if it should also go in to CMSSW_14_2_X. |
|
code-checks |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/43095 Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
Hi @Dominic-Stafford @lviliani @bbilin Looking at the changes from both patches, I see that they are not related at all to the modifications I made. They are issues from the CMSSW_14_2_0_pre3 release (and the code is the same in the CMSSW_15_0_X branch). Do I have to apply both patches even though they have nothing to do with the modifications I made? Manuel González Hernández. |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/44721 Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
manuel-gonzalez-henandez
force-pushed
the
manu_dev
branch
from
19ee914 to
9be668b
Compare
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/44734 Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
|
Hi @manuel-gonzalez-henandez, thanks for all your work on this, it would be nice to get it merged. Could you apply the code checks in the above comment so that the review can proceed, please? |
|
code-checks and apply patch |
|
This PR contains too many commits (370 >= 240) and will not be processed. |
|
Hi @manuel-gonzalez-henandez, I think you're getting this message because you've merged a lot of changes from master, right? It might be possible to get rid of them by rebasing instead of merging if you're confident with git, otherwise could you make a PR from a new branch off master with just your changes, please? |
manuel-gonzalez-henandez
force-pushed
the
manu_dev
branch
from
3214465 to
9be668b
Compare
|
code-checks |
|
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/45026 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-46725/45027 ERROR: Unable to merge PR. See log https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/45027/cms-checkout-topic.log |
|
I was not able to successfully use the patch https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-46725/45026/code-format.patch because the lines that it asked to remove (lines that were already there and have nothing to do with the pull request) were not the same as the ones in the Pythia8Hadronizer.cc from my branch manu_dev. I tried to include the patch's changes myself but now there is a conflict... what should I do? |
PR description:
This pull request proposes a solution to the issue outlined in https://gitlab.cern.ch/cms-gen/gen_tasklist/-/issues/9. The ResonanceDecayFilter was not correctly accounting for the total number of tried events, leading to an incorrect cross section calculation (as the branching ratio was omitted). To address this, I introduced a counter in ResonanceDecayFilterHook.h to track the number of events vetoed by the filter, along with a function to reset the counter after each event. This data is then passed from Pythia8Hadronizer.cc to GenFilterEfficiencyProducer.cc using an object of the class ResonanceDecayFilterCounter I created.
The counter is applied only to the weight counts and not to the event counts, as requested by PDMV, to ensure the event-level GenFilter efficiency excludes the ResonanceDecayFilter information. This exclusion allows PDMV to accurately determine the number of LHE events required to generate a specific number of Gen events.
Additionally, I introduced the input_genfilter_efficiency variable in GenXsecAnalyzer.cc to ensure that when the ResonanceDecayFilter is enabled, the filter efficiency is always calculated using weight counts (where the filter information is stored) instead of event counts. This adjustment ensures consistent efficiency calculations for both LO and NLO processes and includes the filter’s contribution.
Expected changes to the output are as follows:
PR validation:
To verify the correctness of the cross section calculation, I used the LO Radion_GF_HH_M300_narrow gridpack (as referenced in https://gitlab.cern.ch/cms-gen/gen_tasklist/-/issues/9) and a simple NLO ttbar custom gridpack that I generated for testing. In both cases, the resulting cross section was calculated correctly.
I also performed the checks outlined in https://cms-sw.github.io/PRWorkflow.html. Specifically, I executed the runTheMatrix suite of tests, which resulted in four errors in the following workflows:
312.0 Pyquen ZeemumuJets_pt10 2760GeV_2022 (Step0-FAILED)
25202.0 TTbar_13 (Step1-FAILED)
14234.0 TTbar_14TeV+2023FSPU (Step1-FAILED)
250202.181 TTbar13TeVPUppmx2018 (Step1-FAILED)
Each of these failures was due to "No such file or directory" errors caused by missing .root files. To investigate further, I ran the same runTheMatrix tests using an unmodified version of CMSSW_14_2_0_pre3 and observed identical errors. This strongly indicates that the issues are unrelated to my modifications.
Additionally, I adhered to the coding and style guidelines by running the commands scram build code-checks (which reported no diagnostics) and scram build code-format.
If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:
This PR is not a backport.