-
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
[JME,Nano] Refactor constituents table, add GloParT WvsQCD score #47206
[JME,Nano] Refactor constituents table, add GloParT WvsQCD score #47206
Conversation
…tuents. Move jet constituents set up to its own file. Add functions to add AK4 jet and genjets constituents for private production
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47206/43477
|
A new Pull Request was created by @nurfikri89 for master. It involves the following packages:
@cmsbuild, @ftorrresd, @hqucms, @jfernan2, @mandrenguyen can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
enable nano |
please test |
+1 Size: This PR adds an extra 56KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
label btv |
assign btv-pog |
New categories assigned: btv-pog @mondalspandan,@SWuchterl you have been requested to review this Pull request/Issue and eventually sign? Thanks |
@@ -73,6 +73,8 @@ | |||
globalParT3_TopbWmv = Var("bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probTopbWmv')",float,doc="Mass-decorrelated GlobalParT-3 Top->bWmv score",precision=10), | |||
globalParT3_TopbWtauhv = Var("bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probTopbWtauhv')",float,doc="Mass-decorrelated GlobalParT-3 Top->bWtauhv score",precision=10), | |||
globalParT3_QCD = Var("bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probQCD')",float,doc="Mass-decorrelated GlobalParT-3 QCD score.",precision=10), | |||
globalParT3_WvsQCD = Var("?bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probXqq')+bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probXcs')+bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probQCD')>0?(bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probXqq')+bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probXcs'))/(bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probXqq')+bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probXcs')+bDiscriminator('pfGlobalParticleTransformerAK8JetTags:probQCD')):-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.
Isn't this redundant as probXqq
, probXcs
and probQCD
are already stored?
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.
We have had too many incidents where analyzers had wrongly calculated the binarized score from the raw scores. This would avoid the same kind of mistakes in the future.
|
||
process.selectedGenJetAK8Constituents = cms.EDFilter("PATPackedGenParticlePtrSelector", | ||
src = cms.InputTag("genJetAK8Constituents", "constituents"), | ||
cut = cms.string(genJetConstCut) |
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.
cut = cms.string(genJetConstCut) | |
cut = cms.string(genJetAK8ConstCut) |
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.
will fix
name = cms.string(process.genJetTable.name.value()+"GenPartCand"), | ||
candIdxName = cms.string("genPartCandIdx"), | ||
candIdxDoc = cms.string("Index in the GenPartCand table"), | ||
candidates = pfCandidatesTable.src, |
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.
candidates = pfCandidatesTable.src, | |
candidates = process.genPartCandidatesTable.src, |
name = cms.string(process.genJetAK8Table.name.value()+"GenPartCand"), | ||
candIdxName = cms.string("genPartCandIdx"), | ||
candIdxDoc = cms.string("Index in the GenPartCand table"), | ||
candidates = pfCandidatesTable.src, |
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.
candidates = pfCandidatesTable.src, | |
candidates = process.genPartCandidatesTable.src, |
candidates = pfCandidatesTable.src, | ||
jets = process.genJetAK8Table.src, | ||
jetCut = process.genJetAK8Table.cut, | ||
jetConstCut = process.selectedGenJetConstituents.cut |
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.
jetConstCut = process.selectedGenJetConstituents.cut | |
jetConstCut = process.selectedGenJetAK8Constituents.cut |
process = SaveGenJetConstituents(process,True,False) | ||
return process | ||
def SaveGenJetAK8Constituents(process): | ||
process = SaveGenJetConstituents(process,True,False) |
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.
process = SaveGenJetConstituents(process,True,False) | |
process = SaveGenJetConstituents(process,False,True) |
Or better:
process = SaveGenJetConstituents(process,True,False) | |
process = SaveGenJetConstituents(process, addGenJetConst=False, addGenJetAK8Const=True) |
) | ||
process.genjetConstituentsTask.add(process.selectedGenJetAK8Constituents) | ||
|
||
if addGenJetConst or addGenJetConst: |
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.
if addGenJetConst or addGenJetConst: | |
if addGenJetConst or addGenJetAK8Const: |
The first test 4 days ago before the first round of changes indeed looked like a fluctuation but now variations are larger |
@jfernan2 I ran some of the workflows showing the largest differences and got basically identical throughputs w/ and w/o this PR included. What I suspect is that the difference we see here is mainly driven by file access performance... |
It is strange, NANO tests have been reliable until now, both base and PR should be running in the same conditions. @smuzaffar any idea? Thanks in advance |
We see similar fluctuations in #47173 (comment) too -- there are rates change of 20-30% in nano test wfs that are not expected to be affected by that PR... |
But those seem gone after changes in the PR, |
please test @jfernan2 Let's give it another try and see. |
really have no idea, both PR (on cmsbuild956) and baseline (on cmsbuild965) for nano relvals were run on identical nodes. |
+1 Size: This PR adds an extra 16KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
Thank you @smuzaffar |
+1 |
+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. @sextonkennedy, @rappoccio, @mandrenguyen, @antoniovilela (and backports should be raised in the release meeting by the corresponding L2) |
@cms-sw/orp-l2 We would like to get this into |
+1 |
PR description:
This PR refactors the PF constituents table by putting the relevant modules in a dedicated cff file (
jetsConstituents_cff.py
). The main purpose is to streamline the modules used for storing PF constituents information for btvNano. Another PR on revamping btvNano will soon follow. This refactoring will also make it easier for NanoAOD private production with more constituents from other jet collections. In the cff file, I have added two customization functions that can be used to add AK4 and gen-jet constituents. Additionally, the GloParTWvsQCD
binarized score is added to theFatJet
table.PR validation:
runTheMatrix.py -l limited -i all --ibeos
with the exception of a few workflows which failed because of missing relval input files.runTheMatrix.py -i all --ibeos -l 2500.021,2500.022,2500.023,2500.024,2500.031,2500.032,2500.033,2500.034