-
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
Minor fixes to variables for lepton MVA #45754
Conversation
cms-bot internal usage |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45754/41460
|
A new Pull Request was created by @namapane for master. It involves the following packages:
@cmsbuild, @ftorrresd, @hqucms, @vlimant can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45754/41494
|
Pull request #45754 was updated. @cmsbuild, @ftorrresd, @hqucms, @vlimant can you please check and sign again. |
@@ -341,8 +341,9 @@ def _get_bitmapVIDForEle_docstring(modules,WorkingPoints): | |||
miniPFRelIso_all = Var("userFloat('miniIsoAll')/pt",float,doc="mini PF relative isolation, total (with scaled rho*EA PU Winter22V1 corrections)"), | |||
pfRelIso03_chg = Var("userFloat('PFIsoChg')/pt",float,doc="PF relative isolation dR=0.3, charged component"), | |||
pfRelIso03_all = Var("userFloat('PFIsoAll')/pt",float,doc="PF relative isolation dR=0.3, total (with rho*EA PU Winter22V1 corrections)"), | |||
jetRelIso = Var("?userCand('jetForLepJetVar').isNonnull()?(1./userFloat('ptRatio'))-1.:userFloat('PFIsoAll04')/pt",float,doc="Relative isolation in matched jet (1/ptRatio-1, pfRelIso04_all if no matched jet)",precision=8), | |||
jetPtRatio = Var("?userCand('jetForLepJetVar').isNonnull()?userFloat('ptRatio'):-1.",float,doc="ratio of electron pt to the associated jet pt (-1. if none)",precision=10), |
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 seems that PFIsoAll04
is not stored in NanoAOD, so if we make this change we will need to add 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.
Good catch, @hqucms. I assumed that was included in the same way as it it for muons.
But actually, there is somewhat an inconsistency as of now. Electron_jetRelIso
takes PFIsoAll04
for Run3 and
PFIsoAll04_Fall17V2
for Run2, but what is used for LepGood_jetPtRatio in the electronPROMPTMVA
is PFIsoAll04_Fall17V2
in all cases. I suppose that this is because the MVA training was done witth the Fall17V2
version, but then Electron_jetRelIso
is inconsistent with the MVA input for Run3.
I'm unsure on how to proceed then: it depends if the goal is to be able to recover the variable used as input in the electronPROMPTMVA
(then we need to add PFIsoAll04_Fall17V2
), or to be able to recover the current Electron_jetRelIso
(then we have to add PFIsoAll04
for Run3 or PFIsoAll04_Fall17V2
for Run2, but then I miss what's the point of having this variable).
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 current code suggests that one should use PFIsoAll04
for Run3 and PFIsoAll04_Fall17V2
for Run2, so I suppose when the electronPROMPTMVA
is re-trained for Run3 it should move to using PFIsoAll04
?
Maybe @cms-sw/egamma-pog-l2 could comment and clarify on the plans for the update of electronPROMPTMVA
for Run3?
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.
Added pfRelIso04_all
.
I'm neither an author nor an user of the prompt MVA for electrons, but my understanding (from a discussion with Sergio, who can comment better when he will be back) is that there is no such plan of retraining it for run 3 as the improvement would be marginal.
Il giorno 23 ago 2024, alle ore 14:36, Huilin Qu ***@***.******@***.***>> ha scritto:
@hqucms commented on this pull request.
________________________________
In PhysicsTools/NanoAOD/python/electrons_cff.py<#45754 (comment)>:
@@ -341,8 +341,9 @@ def _get_bitmapVIDForEle_docstring(modules,WorkingPoints):
miniPFRelIso_all = Var("userFloat('miniIsoAll')/pt",float,doc="mini PF relative isolation, total (with scaled rho*EA PU Winter22V1 corrections)"),
pfRelIso03_chg = Var("userFloat('PFIsoChg')/pt",float,doc="PF relative isolation dR=0.3, charged component"),
pfRelIso03_all = Var("userFloat('PFIsoAll')/pt",float,doc="PF relative isolation dR=0.3, total (with rho*EA PU Winter22V1 corrections)"),
- jetRelIso = Var("?userCand('jetForLepJetVar').isNonnull()?(1./userFloat('ptRatio'))-1.:userFloat('PFIsoAll04')/pt",float,doc="Relative isolation in matched jet (1/ptRatio-1, pfRelIso04_all if no matched jet)",precision=8),
+ jetPtRatio = Var("?userCand('jetForLepJetVar').isNonnull()?userFloat('ptRatio'):-1.",float,doc="ratio of electron pt to the associated jet pt (-1. if none)",precision=10),
The current code suggests that one should use PFIsoAll04 for Run3 and PFIsoAll04_Fall17V2 for Run2, so I suppose when the electronPROMPTMVA is re-trained for Run3 it should move to using PFIsoAll04?
Maybe @cms-sw/egamma-pog-l2<https://github.com/orgs/cms-sw/teams/egamma-pog-l2> could comment and clarify on the plans for the update of electronPROMPTMVA for Run3?
—
Reply to this email directly, view it on GitHub<#45754 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ABHN4UDZUJ73ITUIWHRLOIDZS4UFVAVCNFSM6AAAAABM2K6SLWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDENJXGEZDQNBWGQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Hi, Thanks @namapane for looking into this and for the proposal. I was indeed looking at the fact that with the current solution we'd essentially drop In any case it would seems strange to me to have |
Thanks @RSalvatico for this clarification.
Thinking a bit more on this, I would say that storing the "Run3" version of input variables makes sense as it will allow to retrain the MVA and recompute it over the new training, if aybody ever wants to. In the meanwhile, the existing So I would stick to @hqucms 's initial suggestion and add only |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45754/41539
|
Pull request #45754 was updated. @cmsbuild, @ftorrresd, @hqucms, @vlimant can you please check and sign again. |
The latest commit introduces |
I think @RSalvatico and/or other egamma experts should check carefully all electron-related diffs; then, if everybody is OK with the proposal, I can turn the draft PR into a regular one. |
enable nano |
please test |
+1 Size: This PR adds an extra 12KB to repository Comparison SummarySummary:
NANO Comparison SummarySummary:
Nano size comparison Summary:
|
Milestone for this pull request has been moved to CMSSW_14_2_X. Please open a backport if it should also go in to CMSSW_14_1_X. |
I opened #45860 as requested and will close this one. |
PR description:
We recently realized that the inputs for the muonPROMPTMVA (and likewise for electronPROMPTMVA) are almost, but not fully recoverable from nanoAODs. This means that it is not possible to check data/MC agreement for input variables from central productions, nor to test new trainings.
This can be fixed easily and cheaply with two small changes (using muons for illustration, same applies to electrons):
Muon_jetDF
, corresponding to theLepGood_jetDF
MVA inputMuon_jetRelIso
with respect to the variable that is intended to correspond to,LepGood_jetPtRatio
. With the current definition,jetRelIso = (1/ptRatio-1)
if a jet is present,pfRelIso04_all
otherwise. The(1/x-1)
transformation was intended to save space. Unfortunately the actual MVA input variable is defined in a slightly different way, with amax(ptRatio, 1.5)
applied in the case a jet is associated. Since it is not possible to figure out unambiguously if this was the case, recovering the exact definition ofLepGood_jetPtRatio
that was used for the MVA is tricky.Our proposal is to replace
Muon_jetRelIso
with the plainptRatio
, without a transformation, and with a default of -1 if no jet is matched. This has some advantages:ptRatio
withpfRelIso04_all
, which is already available in its own variable. This improves clarity and also saves some disk space as -1 gets compressed betterMuon_jetRelso
as defined now, it can be computed easily as well (but I doubt there is much use of it apart for studies related to the MVA, for which having the plainptRatio
is more handy anyhow)Muon_jetPtRatio
explicitly)Electron
variables. In this case,Electron_pfRelIso04_all
is also added since this variable, used inElectron_jetRelso
, was not present.PR validation:
(1/x-1)
transformation does not increase the rounding error, compared the rounding error of derivingptRatio
from the curentMuon_jetPtRatio
with the one of storingptRatio
directly. We found that precision=10 gives a smaller rounding error while still reducing the space from 2.4 b/muon to 2.2 b/muon (this is because we fill -1 instead ofpfRelIso04_all
when no jet is found).Electron_jetPtRatio
takes 2.3 b/item instead of 2.4 b/item ofElectron_jetRelIso
.Electron_pfRelIso04_all
costs 2.5 b/itemMuon_jetDF
andElectron_jetDF
cost 2.2 b/item each