-
Notifications
You must be signed in to change notification settings - Fork 4.7k
[NGT] Add workflow to produce Phase2 NANOAOD with L1 and HLT objects #51004
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
base: master
Are you sure you want to change the base?
Changes from 3 commits
0d5b238
b6de68e
74ef3bd
31b674f
2cbc11f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import FWCore.ParameterSet.Config as cms | ||
|
|
||
| # This modifier is for running NANOAOD production from L1 and HLT steps, | ||
| # whitout running RECO and PAT steps. | ||
| # It excludes all GEN sequences that depend on PAT collections. | ||
|
|
||
| nano_l1_hlt = cms.Modifier() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,12 @@ def addPh2GTObjects(process): | |
| from DPGAnalysis.Phase2L1TNanoAOD.l1tPh2Nanotables_cff import * | ||
| def addPh2L1Objects(process): | ||
| process.l1tPh2NanoTask.add(p2L1TablesTask) | ||
|
|
||
| # This modifier excludes the hpsTauTable, which is based on the l1tHPSPFTauProducerPuppi collection, no longer available in the L1 menu. | ||
| # This allows to run the NANOAOD production from L1 and HLT steps, whitout assuming old inputs from the Spring24 datasets. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a nitpick: what do you mean by "assuming" here? Maybe "considering" or "assuming the existence"? Same below.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case you change something else (otherwise ignore this comment): there is also a typo in "whitout". |
||
| from Configuration.ProcessModifiers.nano_l1_hlt_cff import nano_l1_hlt | ||
| nano_l1_hlt.toReplaceWith(p2L1TablesTask, p2L1TablesTask.copyAndExclude([hpsTauTable])) | ||
|
|
||
| return process | ||
|
|
||
| #### GENERATOR INFO | ||
|
|
@@ -25,6 +31,8 @@ def addPh2L1Objects(process): | |
| from PhysicsTools.NanoAOD.taus_cff import * ## for Gen taus | ||
| def addGenObjects(process): | ||
|
|
||
| process.genNanoTask = cms.Task() | ||
|
|
||
| ## add more GenVariables | ||
| # from L1Ntuple Gen: https://github.com/artlbv/cmssw/blob/94a5ec13b8ce76afb8ea4f157bb92fb547fadee2/L1Trigger/L1TNtuples/plugins/L1GenTreeProducer.cc#L203 | ||
| genParticleTable.variables.vertX = Var("vertex.x", float, "vertex X") | ||
|
|
@@ -45,22 +53,35 @@ def addGenObjects(process): | |
| process.prunedGenParticleTable = genParticleTable.clone() | ||
| process.prunedGenParticleTable.src = "prunedGenParticles" | ||
| process.prunedGenParticleTable.name = "prunedGenPart" | ||
| process.l1tPh2NanoTask.add(process.prunedGenParticleTable) | ||
| process.genNanoTask.add(process.prunedGenParticleTable) | ||
|
|
||
| # lower genVisTau pt threshold | ||
| process.genVisTauTable.cut = "pt > 1" | ||
| # lower AK8 gen jet threshold | ||
| process.genJetAK8Table.cut = "pt > 10" | ||
|
|
||
| process.l1tPh2NanoTask.add( | ||
| process.genNanoTask.add( | ||
| puTable, metMCTable, | ||
| genParticleTask, genParticleTablesTask, | ||
| genTauTask, | ||
| ) | ||
|
|
||
| # add all GenJets: AK4 and AK8 | ||
| process.l1tPh2NanoTask.add(genJetTable,patJetPartonsNano,genJetFlavourTable) | ||
| process.l1tPh2NanoTask.add(genJetAK8Table,genJetAK8FlavourAssociation,genJetAK8FlavourTable) | ||
| process.genNanoTask.add(genJetTable,patJetPartonsNano,genJetFlavourTable) | ||
| process.genNanoTask.add(genJetAK8Table,genJetAK8FlavourAssociation,genJetAK8FlavourTable) | ||
|
|
||
| process.l1tPh2NanoTask.add(process.genNanoTask) | ||
|
|
||
| # This modifier excludes all GEN sequences that depend on PAT collections. | ||
| # This allows to run the NANOAOD production from L1 and HLT steps, whitout assuming the PAT inputs. | ||
| from Configuration.ProcessModifiers.nano_l1_hlt_cff import nano_l1_hlt | ||
| nano_l1_hlt.toReplaceWith(process.genNanoTask, | ||
| process.genNanoTask.copyAndExclude( | ||
| [puTable, | ||
| metMCTable, | ||
| genJetAK8Table, | ||
| genJetAK8FlavourAssociation, | ||
| genJetAK8FlavourTable])) | ||
|
|
||
| return process | ||
|
|
||
|
|
||
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.
Thinking about manual configuration, will this step run if the user forgets to specify
@Phase2L1DPGwithGen, despite havingnano_l1_hltactive?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.
why would a user do that? Following wrong recipes results typically in wrong results, no?
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.
Sure; I was just suggesting that adding some protection might help.
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.
The
NANO:@NGTScoutingVal+@Phase2L1DPGwithGenstep should be the only ingredient required to produce the L1+HLT NanoAOD sample.The
nano_l1_hltprocModifier is introduced as a protection against missing PAT collections that are required in some of the L1Nano sequences. The alternative would have been to reorganize the L1Nano configuration itself; however, this would have modified the standard L1Nano format used by existing L1 workflows and users, which I wanted to avoid.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.
@elenavernazza It is clear what
nano_l1_hltachieves. My comment was rather addressing future users that might use this new process modifier without fully knowing the required inputs. I don't know what happens if the user specifies onlyNANO:@NGTScoutingVal, but the potential crash might be cryptic. I was suggesting you could add some checks / protections that throw a meaningful error message if the user forgets to add@Phase2L1DPGwithGen.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.
No, why should it?
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.
Please see my comment.
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.
by this token, should you test all possible workflow combinations in the matrix ?
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.
I never suggested this.
I tested
NANO:@NGTScoutingValwithnano_l1_hlt, since the bot does not test it, and since I thought that it could lead to a crash, due to the absence of@Phase2L1DPG.There is no crash, so all is well.
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.
It is straightforward to see from the code changes that this should not lead to a runtime crash.
The only modifications affecting
NANO:@NGTScoutingValare inHLTrigger/NGTScouting/python/HLTNanoProducer_cff.pyand are limited to:dstTriggerAcceptFilterfilter for the production of the HLT tablesAs already stated in the PR description, these changes simply allow all events to be written to the output NanoAOD, independently of the L1/HLT decision. No producer dependencies or runtime execution paths are modified in a way that could plausibly introduce a crash.
Because of that, I am a bit puzzled that this specific combination of steps/process modifiers was singled out in the review, while there are already many other untested combinations present in CMSSW that would be equally (or more) likely to expose runtime issues.