-
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
[NGT] Implement NanoAOD
ntuples production for HLT Muon objects
#47069
base: master
Are you sure you want to change the base?
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47069/43241 |
A new Pull Request was created by @Parsifal-2045 for master. It involves the following packages:
@AdrianoDee, @Moanwar, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @ftorrresd, @hqucms, @mandrenguyen, @miquork, @rappoccio, @srimanob, @subirsarkar can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
type ngt |
enable nano |
please test |
@@ -702,6 +702,7 @@ def SwapKeepAndDrop(l): | |||
phase2_muon.toModify(FEVTDEBUGHLTEventContent, | |||
outputCommands = FEVTDEBUGHLTEventContent.outputCommands + [ | |||
'keep recoMuons_muons1stStep_*_*', | |||
'keep *_hltL2OfflineMuonSeeds_*_*', |
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.
does this need to be saved regardless or can it be (for the time being) gated through the phase2L2AndL3Muons
modifier?
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's actually the other way around. The collection is not produced when using the phase2L2AndL3Muons
procModifier, but I decided to produce ntuples also for the both the current muon seeds collections, namely the "offline" one and the one from L1Tk Muons (at least for 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.
The collection is not produced when using the phase2L2AndL3Muons procModifier,
I then suggest to apply the reverse modifier, that way it will be easier to remove once it's re-absorbed.
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 collection is not produced when using the phase2L2AndL3Muons procModifier,
I then suggest to apply the reverse modifier, that way it will be easier to remove once it's re-absorbed.
What's the correct syntax for that? I now have (phase2Muon and not phase2L3AndL3Muons).toModify...
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.
What's the syntax for that?
(phase2Muon & ~phase2L2AndL3Muons).toModify(...)
(OTOH, I am not formally a reviewer of this PR, so I let the @cms-sw/operations-l2 decide what's best -- incidentally ever increasing the size of FEVTDEBUGHLTEventContent
will inevitably lead to operation issues when trying to run big MC samples production )
+1 Size: This PR adds an extra 60KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
+Upgrade |
from DPGAnalysis.MuonTools.dtSegmentFlatTableProducer_cfi import dtSegmentFlatTableProducer | ||
|
||
hltDtSegmentFlatTable = dtSegmentFlatTableProducer.clone( | ||
name = "dtSegment", | ||
src = "hltDt4DSegments", | ||
doc = "DT segment information", | ||
variables = cms.PSet( | ||
seg4D_hasPhi = Var("hasPhi()", bool, doc = "has segment phi view - bool"), | ||
seg4D_hasZed = Var("hasZed()", bool, doc = "has segment zed view - bool"), | ||
seg4D_posLoc_x = Var( | ||
"localPosition().x()", float, doc = "position x in local coordinates - cm" | ||
), | ||
seg4D_posLoc_y = Var( | ||
"localPosition().y()", float, doc = "position y in local coordinates - cm" | ||
), | ||
seg4D_posLoc_z = Var( | ||
"localPosition().z()", float, doc = "position z in local coordinates - cm" | ||
), | ||
seg4D_dirLoc_x = Var( | ||
"localDirection().x()", float, doc = "direction x in local coordinates" | ||
), | ||
seg4D_dirLoc_y = Var( | ||
"localDirection().y()", float, doc = "direction y in local coordinates" | ||
), | ||
seg4D_dirLoc_z = Var( | ||
"localDirection().z()", float, doc = "direction z in local coordinates" | ||
), | ||
seg2D_phi_t0 = Var( | ||
f"? hasPhi() ? phiSegment().t0() : {defaults.FLOAT}", | ||
float, | ||
doc = "t0 from segments with phi view - ns", | ||
), | ||
seg2D_phi_nHits = Var( | ||
f"? hasPhi() ? phiSegment().specificRecHits().size() : 0", | ||
"int16", | ||
doc = "# hits in phi view - [0:8] range", | ||
), | ||
seg2D_phi_vDrift = Var( | ||
f"? hasPhi() ? phiSegment().vDrift() : {defaults.FLOAT_POS}", | ||
float, | ||
doc = "v_drift from segments with phi view", | ||
), | ||
seg2D_phi_normChi2 = Var( | ||
f"? hasPhi() ? (phiSegment().chi2() / phiSegment().degreesOfFreedom()) : {defaults.FLOAT_POS}", | ||
float, | ||
doc = "chi2/n.d.o.f. from segments with phi view", | ||
), | ||
seg2D_z_t0 = Var( | ||
f"? hasZed() ? zSegment().t0() : {defaults.FLOAT}", | ||
float, | ||
doc = "t0 from segments with z view - ns", | ||
), | ||
seg2D_z_nHits = Var( | ||
f"? hasZed() ? zSegment().specificRecHits().size() : 0", | ||
"int16", | ||
doc = "# hits in z view - [0:4] range", | ||
), | ||
seg2D_z_normChi2 = Var( | ||
f"? hasZed() ? (zSegment().chi2() / zSegment().degreesOfFreedom()) : {defaults.FLOAT_POS}", | ||
float, | ||
doc = "chi2/n.d.o.f. from segments with z view", | ||
) | ||
), | ||
detIdVariables = cms.PSet( | ||
wheel = DetIdVar("wheel()", "int16", doc = "wheel - [-2:2] range"), | ||
sector = DetIdVar( | ||
"sector()", | ||
"int16", | ||
doc = "sector - [1:14] range" | ||
"<br />sector 13 used for the second MB4 of sector 4" | ||
"<br />sector 14 used for the second MB4 of sector 10", | ||
), | ||
station = DetIdVar("station()", "int16", doc = "station - [1:4] range") | ||
), | ||
globalPosVariables = cms.PSet( | ||
seg4D_posGlb_phi = GlobGeomVar( | ||
"phi().value()", doc = "position phi in global coordinates - radians [-pi:pi]" | ||
), | ||
seg4D_posGlb_eta = GlobGeomVar("eta()", doc = "position eta in global coordinates") | ||
), | ||
globalDirVariables = cms.PSet( | ||
seg4D_dirGlb_phi = GlobGeomVar( | ||
"phi().value()", | ||
doc = "direction phi in global coordinates - radians [-pi:pi]", | ||
), | ||
seg4D_dirGlb_eta = GlobGeomVar( | ||
"eta()", doc = "direction eta in global coordinates" | ||
) | ||
) | ||
) |
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 am also not a formal reviewer of this PR, but I suggest re-using existing configuration snippets when available.
E.g. all this could become:
from DPGAnalysis.MuonTools.nano_mu_local_reco_cff import dtSegmentFlatTable as dtSegmentFlatTable_
hltDtSegmentFlatTable = dtSegmentFlatTable_.clone(
name = "dtSegment",
src = "hltDt4DSegments",
doc = "DT segment information"
)
) | ||
) | ||
|
||
from DPGAnalysis.MuonTools.gemRecHitFlatTableProducer_cfi import gemRecHitFlatTableProducer |
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 same comment made for hltDtSegmentFlatTable
also holds for GEM.
) | ||
) | ||
|
||
from DPGAnalysis.MuonTools.rpcRecHitFlatTableProducer_cfi import rpcRecHitFlatTableProducer |
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 same comment made for hltDtSegmentFlatTable
also holds for RPC.
) | ||
|
||
# L2 standalone muons updated at vertex | ||
l2MuTableVtx = cms.EDProducer( |
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.
A clone
- based approach (cloning from l2MuTable
) should also work for all SimpleTriggerTrackFlatTableProducer
objects.
Overall, you are always preserving the same structure of variables.
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.
You could consider renaming the file to muNtupleProducerHlt_cff.py
to preserve the convention used elsewhere in this folder.
Include HLT L2 Muon offline seeds in the event content (when necessary) to compare them with the ones from L1Tk Muons
f94c6ee
to
1838003
Compare
All suggested changes have been implemented and I've verified that |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-47069/43437
|
Pull request #47069 was updated. @AdrianoDee, @Moanwar, @antoniovilela, @cmsbuild, @davidlange6, @DickyChant, @fabiocos, @ftorrresd, @hqucms, @mandrenguyen, @miquork, @rappoccio, @srimanob, @subirsarkar can you please check and sign again. |
please test |
+1 Size: This PR adds an extra 36KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
PR description:
This PR allows to produce, on-demand, ntuples in
NanoAOD
format for all HLT muon objects, including L1TkMuons and all the new collections introduced by #46897.The production is triggered automatically when running .777 or .778 workflows. Otherwise, it can be triggered by adding
NANO:@MUHLT
as a step in any step2cmsDriver
command, together with the addition ofNANOAODSIM
in bothdatatier
andeventcontent
.PR validation:
No physics changes are foreseen.
I've tested on the usual 29834 and 29850 workflows also in their .777 and .778 flavours, in all cases the workflows run and ntuples are produced correctly
.0 workflows require to manually trigger the nutples production using, for example: