-
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
Fix calculation of smeared MET on MiniAOD #36165
Conversation
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36165/26718
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36165/26719
|
A new Pull Request was created by @michaelwassmer for master. It involves the following packages:
@jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
@michaelwassmer How old is this problem and would a backport be needed? |
@cmsbuild please test |
Here is a link to a presentation in the JetMET meeting comparing original and fixed calculation: https://indico.cern.ch/event/1048658/contributions/4405700/attachments/2263616/3842703/SmearedMET_Michael_Wassmer.pdf |
-1 Failed Tests: HeaderConsistency Comparison SummarySummary:
|
@cmsbuild please test |
+1 Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6a446a/21407/summary.html Comparison SummarySummary:
|
@michaelwassmer @jpata I was directed here from the newly introduced critical bugfix, as we have both edited the .cc file of pat::Jet. This fix seems to be a very welcome one, but I still needed to go through the whole code patch to check that there are no underlying conflicts. What I'm slightly worried about here is that quite a lot of new code have been introduced for the seemingly simple problem of undoing and redoing the jer smearing. If I understood correctly, the main point is to be able to undo the smearing scale after jet.correctedP4("Uncorrected") is called in this code. Has it been recognized that this issue could have been solved with lots of less new code by simply calling jet.correctedP4("Unscaled") instead of jet.correctedP4("Uncorrected") and leaving the new parts with jet.jerFactor() out? From 10_6_X onwards, the pat::Jet::scaleEnergy function appends any additional jet energy scales by the title "Unscaled" in to the beginning of the JEC level vector. This function is most of all utilized by the jet smearing routines. In short: if smearing is applied to a pat::Jet using the scaleEnergy function, one can undo the JECs and JER smearing simultaneously by referring to the "JEC level" "Unscaled" instead of "Uncorrected". In addition, if any additional energy scales have been applied, referring to "Unscaled" in correctedP4() automatically refers to the point where all energy scales are undone. In this sense, using "Unscaled" and the already existing structures is a more universal solution than the one proposed here. The only "additional" check that would be needed, is checking if the "lowest" available correction level is "Unscaled" or "Uncorrected". Tagging also @ahinzmann , who was involved in introducing the "Unscaled" correction level in the first place. EDIT: reading the comments with a deepened thought, the issue is probably deeper and a universal behavior also for non-pat::Jet's might probably require the sorts of additions as introduced here. However, the question and standpoint remain: how much could this PR be shortened by the usage of jet.correctedP4("Unscaled"). The complexity and deep inheritance of the current CMSSW build are already a problem when bugtracking and development are considered. Hence I think it's strongly encouraged to vigilantly minimize the amount of new code introduced in any new PR's. |
Hi @errai- |
Hi @michaelwassmer and thanks for the prompt reply! The level "Uncorrected" is introduced when the pat::Jet is created in association to the jet energy corrections. Actually I'm not exactly sure where the code for this is located (maybe @ahinzmann can help?). I think it's best to give a brief example: take a pat::Jet where the JEC level is set to L3Absolute. The jet corrections (e.g. for correctedP4()) are all called through the function jecFactor, which looks for the corrections within an ordered vector. Simplifying a bit, as the jet is created, the ordered return values of jecFactor look like this: Uncorrected=0.959391, L1FastJet=0.944361, L2Relative=1, L3Absolute=1 If one would like to swap e.g. to the level L1FastJet, one would need to multiply the current P4 by 0.944361. If one would like to move to L2Rel or L3Abs, the scale is simply 1. (as L3Absolute is a unitary dummy scaling). Here, the level Uncorrected (still) refers to the completely raw jet. The jer smearing is "postulated" onto a jet which already has the jet corrections. After the smearing, the (vector) of jecFactor return values will look like this: Unscaled=0.936635, Uncorrected=0.959391, L1FastJet=0.944361, L2Relative=1, L3Absolute=1 The factor of the smearing factor is added into the beginning of this vector! This changes the meaning of "Uncorrected": it now refers to the jet with no jet corrections, but smearing is still applied. Calling "Unscaled" gives the completely uncorrected and unsmeared jet. Just as a reminder, it is also possible that scaleEnergy is called in other purposes than jet smearing. If one would call the function with a unitary dummy scale before smearing, one would end up with Unscaled=0.936635, Unscaled=0.959391, Uncorrected=0.959391, L1FastJet=0.944361, L2Relative=1, L3Absolute=1 That is, there is a dummy "Unscaled" on the left-hand-side of "Uncorrected". However, calling correctedP4("Unscaled") will return the leftmost "Unscaled" case. Note: it is for now recommended to avoid any extra calls to scaleEnergy, as this is related to the bugfix. In MC at L3Abs/L2L3Rel a single call of scaleEnergy from within jet smearing is safe, though. As an example and continuing with these guidelines, one could replace this code reapplying the jer scale on a raw jet by adding something like the following into the jetcorrextractor file:
|
@jpata @michaelwassmer I have confirmed with @kirschen that we would like this fix to be available for UL analyses and therefore should also have a backport. |
@camclean thanks for the confirmation. @michaelwassmer @errai- thanks for the discussion on the implementation. I'm not an expert on JME corrections code as such, so I leave it to JME to discuss thoroughly and propose the best implementation. Clearly, if something related ("Unscaled") already exists and we can avoid introducing more layers of complexity, it should be preferred. I could suggest to discuss this next year with all parties involved (at e.g. the JME meeting) before considering this PR for merging. |
Thanks for the comment @jpata ! I gave @kirschen a quick ping about the topic, in case he hasn't started his holidays yet. |
kind ping on this. will there be a discussion in JME about the implementation? |
Just my 2 cents. I have no strong opinion whether to implement a dedicated interface for JER in pat::Jet, or keep the current (which uses "unscaled"). One way to address the bug may to replace correctedP4("uncorrected") by correctedP4(0) where necessary. This appears in several places in CMSSW and it's obvious, also how to keep MiniAOD and NanoAOD in sync: |
kind ping @michaelwassmer, has there been any discussion on the path forward in JME? |
Hi, |
Just as a quick comment: there might be a slight preference towards @ahinzmann 's correctedP4(0) for two reasons:
Admittedly, correctedP4(0) is not very readable/clean code, so that's the downside one would need to accept. |
Thanks @errai-
Then from the residuals of the L1 collection and the L1L2L3 collection it calculates the Type1 MET corrections.
implicitly assumes that the only "difference" between jecFactor("Uncorrected") and jetFactor("Unscaled") (or jecFactor(0)) is the smearing correction. I guess this could be problematic if for any reason the scaleEnergy method was invoked in addition to the one time during the smearing procedure because then one would not be able to get the smearing factor from this ratio. Am I missing something? |
@michaelwassmer all scaleEnergy calls go into the same 'box' with jet smearing, accumulating the total jet energy scale. That is, if one adds extra scales to jets e.g. for the purpose of systematic variations, one needs to consider them in MET calculus exactly in the same manner as the jet smearing scales. Thus for MET, I see no reason of separating smearing scales from other scaleEnergy call types. One could think of all the non-JEC scales as one single generic extra scale that needs to be considered. Does this answer your question, or is there some subtle detail I missed? Finally, to underline the features of the code: the level "Uncorrected" always exists, but "Unscaled" only exists if scaleEnergy has been called e.g. in the purpose of jet smearing. Hence, jecFactor(0) is the safest and most universal call one can make, as it corresponds to "Uncorrected" in the cases where "Unscaled" is not available for some reason. |
I created a new PR (#36797) in which I incorporated the suggested changes. I thought it would probably be cleaner to have a new PR since this one is already quite long. |
-reconstruction |
@cmsbuild please close |
PR description:
This pull request fixes the incorrect calculation of the nominal smeared missing transverse energy (MET). The underyling problem is the application of the JER correction factors. These factors are applied in PhysicsTools/PatUtils/interface/SmearedJetProducerT.h but later not considered when reverting to the uncorrected jets in JetMETCorrections/Type1MET/interface/PFJetMETcorrInputProducerT.h.
This results in the problem that the JER correction factors in the smeared MET calculation effectively cancels and therefore the smeared MET is basically the same as the regular type1-corrected MET. After the changes shown below, the nominal smeared MET distributions from MiniAOD are similar to the ones obtained from NanoAOD.
This pull request includes the following:
PhysicsTools/PatUtils/plugins/SmearedJetProducer.cc
PhysicsTools/PatUtils/interface/SmearedJetProducerT.h
PhysicsTools/PatUtils/plugins/PATPFJetMETcorrInputProducer.cc
JetMETCorrections/Type1MET/interface/PFJetMETcorrInputProducerT.h
Finally, there is also a mistake when saving the JERup and JERdown varied smearedMET. The different contributions of the nominal smeared MET correction and the JERup and JERdown variations are not separated correctly from on another. After the changes shown below, the varied smeared MET distributions from MiniAOD are similar to the ones obtained from NanoAOD.
DataFormats/PatCandidates/src/MET.cc
PR validation: