-
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
Adding chsMET and trackMET to miniAOD #20655
Changes from 5 commits
046cf84
ce18926
79b2107
8aaf704
123a086
07f5ac9
b226afb
159dd89
d81d212
d139b77
28f06fe
aab6709
bd91dae
7fb017a
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 |
---|---|---|
|
@@ -172,6 +172,41 @@ def miniAOD_customizeCommon(process): | |
del process.slimmedMETsNoHF.caloMET | ||
# ================== NoHF pfMET | ||
|
||
# ================== CHSMET | ||
process.CHSCands = cms.EDFilter("CandPtrSelector", | ||
src=cms.InputTag("packedPFCandidates"), | ||
cut=cms.string("fromPV(0) > 0") | ||
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. following up on today's discussion and last week mail thread, are we sure we want to use this definition for TkMET? how about using this definition 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. given that we do not have a better definition right now (studies started), we keep it as is, and we can systematically update when we decide. This definition is hand in hand correlated with what jets and is used in the JEC computation. therefore the met has to be built from the pf candidates identical to what jets are reconstructed with. |
||
) | ||
task.add(process.CHSCands) | ||
|
||
process.pfMetCHS = cms.EDProducer("PFMETProducer", | ||
src = cms.InputTag("CHSCands"), | ||
alias = cms.string('pfMet'), | ||
globalThreshold = cms.double(0.0), | ||
calculateSignificance = cms.bool(False), | ||
) | ||
|
||
task.add(process.pfMetCHS) | ||
|
||
# ================== CHSMET | ||
|
||
# ================== TrkMET | ||
process.TrkCands = cms.EDFilter("CandPtrSelector", | ||
src=cms.InputTag("packedPFCandidates"), | ||
cut=cms.string("fromPV(0) > 0 && charge()!=0") | ||
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. @zdemirag I meant to comment on this one (tkMET) not on the one above (chsMET), for this I do not see any reason to be consistent with CHS, we should rather have something that has the features expected by the users (i.e. stability against PU) hence we could use a tighter PV association such as 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. ... or perhaps even 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. ah thats fine then. I thought you were referring to CHS. Give me (tops) one-two days, I have preliminary studies that can back up the decision we make. but in general, I see no problem updating this portion of the code in this PR and will update. 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. i'll move the definition to ==> "charge()!=0 && pvAssociationQuality()>=4 && vertexRef().key()==0" , it is because >=4 has slightly lower energy removal for barrel jets (~5%) at all pts in a non-pu sample. 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. 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. I guess mini was updated after nano was already implemented |
||
) | ||
task.add(process.TrkCands) | ||
|
||
process.pfMetTrk = cms.EDProducer("PFMETProducer", | ||
src = cms.InputTag("TrkCands"), | ||
alias = cms.string('pfMet'), | ||
globalThreshold = cms.double(0.0), | ||
calculateSignificance = cms.bool(False), | ||
) | ||
|
||
task.add(process.pfMetTrk) | ||
# ================== TrkMET | ||
|
||
## PU JetID | ||
process.load("RecoJets.JetProducers.PileupJetID_cfi") | ||
task.add(process.pileUpJetIDTask) | ||
|
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.
this seems unsafe.
If variables should not be filled, the configuration should be set to not do it (e.g. empty labels).
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 issues is, we wanted to make the new track met floats a function of patMET to be called directly from miniaod (instead of user floats in miniaod, since it was discouraged). this is the same strategy as the calo met usage at the moment. however, unlike calo met, since the track met is not defined in reco/aod, the pat configurations fail. in the miniaod we now explicitly embed the new met producers for track met and chs met. if you have suggestions happy to work on 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.
If I understood correctly, this piece (if isValid) is addressing an issue in the PAT configuration which is not miniAOD.
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.
thank you for your suggestion, appreciate it a lot. We went ahead with the chsmetSource "" by default option, and edited the miniAOD tools accordingly to read the correct values.
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.
although now everything "works" and achieves the goals, this really is not pretty. After discussing with Matthieu, he suggested that we do:
then for our current purpose
Is this plan acceptable from the point of view of the changes in the format (assuming we will comply with the time requirements)?
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'm not sure I follow the proposal. If the idea is to add something to the RECO step so that it is in RECO/AOD, then we may have to wait for relvals to appear first to then make the next step and add relevant producers to miniAOD that will read these new products from RECO/AOD.
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.
propagating the changes on reco/aod to miniaod level is for the next time per say.. this does not have direct implications on the approach of adding it to pat met slimmer. although given we are working on this at the moment, we can add the reco component as well so we can re-optimize/minimize the code next time (eliminating the usage of EDProducer in miniaod tool)? but we can decouple the two discussions as well