-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Add DY option to Embedding HepMC filter #38829
Conversation
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38829/31201
|
A new Pull Request was created by @IzaakWN (Izaak) for master. It involves the following packages:
@SiewYan, @mkirsano, @Saptaparna, @cmsbuild, @GurpreetSinghChahal, @alberto-sanchez can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
@IzaakWN , do you happen to have a test snippet included with this PR to test the switch to this filter? |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0ff0c5/26427/summary.html Comparison SummarySummary:
|
Sure, I grabbed the fragment for
and simply added
and then run with something like
You can find more instruction in the back up of this presentation: |
Thanks you for the slide, it looks fine for me. |
please test |
@cms-sw/generators-l2 Do you have any comment? |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0ff0c5/26553/summary.html Comparison SummarySummary:
|
Just for the record, I have already opened these two backports:
|
} | ||
if (mom_id == ZPDGID_) { | ||
isHardProc = true; // intermediate boson | ||
} else if (includeDY_ && 11 <= pdg_id && pdg_id <= 16 && mcTruthHelper_.isFirstCopy(**particle) && |
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.
11 <= pdg_id && pdg_id <= 16
: instead of doing this, do you want to select the pdg_ids explicitly. Do you care about Z
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.
In principle this filter only targets the charged leptons, but this is done a few lines below and selecting 11, 13 and 15 explicitly should make no significant difference in performance.
I can change it if you really want?
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.
@Saptaparna, are you fine with leaving it as it is now?
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.
Yes, sorry. It is fine. Just signed PR.
please test |
@cms-sw/generators-l2 Any comment? |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0ff0c5/26821/summary.html Comparison SummarySummary:
|
+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. @perrotta, @dpiparo, @qliphy, @rappoccio (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
Hi @IzaakWN, |
@qliphy @aandvalenzuela, I guess it is because the cmssw/TauAnalysis/MCEmbeddingTools/python/EmbeddingPythia8Hadronizer_cfi.py Lines 8 to 24 in 6d2f660
Is it possible to make it optional here?
|
@IzaakWN the optimal solution would be to add a A quicker fix would be to simply add the missing parameter in cmssw/TauAnalysis/MCEmbeddingTools/python/EmbeddingPythia8Hadronizer_cfi.py, assigning it a "false" default value. Could you please take care of it? |
@IzaakWN in your fix please both update EmbeddingPythia8Hadronizer_cfi.py and assign a default value for "IncludeDY" as you are suggesting |
Hi @qliphy, @aandvalenzuela, @perrotta, the fix is in PR #39090 and it has been pushed to the backports as well. Apologies for the troubles. |
PR description
This PR adds the
IncludeDY
option to theEmbeddingHepMCFilter
in order to allow for the inclusion of Drell-Yan MC events with virtual photons / Z bosons for which the intermediate boson is missing in the particle collection, e.g.p p > ta ta~
). For example, this is needed for correctly filtering events of theDYJetsToTauTauToMuTauh_M-50
sample. The proposed changes were discussed in the TauPOG here: https://indico.cern.ch/event/1170879/#2-validation-of-exclusive-dy-tWe have also added the absolute value function around the mother's PDG ID to allow for charged bosons, such as W (±24) for H → WW.
PR validation
scram build code-checks
andscram build code-format
.PR backporting
After PR, we will request to backport these changes to
CMSSW_10_6_X
as well, so we can regenerate theDYJetsToTauTauToMuTauh_M-50
sample for Run-2 UL MC for TauPOG studies, and also to the CMSSW version for the future Run-3 MC campaign.