Skip to content
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

Merging jet uncertainty evaluator #114

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

quark2
Copy link
Contributor

@quark2 quark2 commented Nov 20, 2018

Launching jet uncertainty evaluator, and modifying the object selection code for adjusting this efficiently

@watson-ij
Copy link
Contributor

Could you give a brief writeup of what each piece is doing and where and how it runs? In particular, we have to do this at the nano level in CMSSW, it needs something not already in the nanoAOD?

@quark2
Copy link
Contributor Author

quark2 commented Nov 20, 2018

Ah, sorry, it's my fault. I should describe the details...
As you see, the edm class JetUncertaintyEvaluator has two parts; JES part (see [1]) and JER part (see [2]), which are from [3] and [4], respectively. They calculate JER and JES uncertainty evaulation factors in each of up and down, and save them in Jet_jes_up, Jet_jes_dn, Jet_jer_up, Jet_jer_dn in nanoAOD ntuples. A user who wants to study uncertainties from JER and JES can use them by replacing Jet_mass and Jet_pt by Jet_mass * Jet_jes_up and Jet_pt * Jet_jes_up, and similar for jes_dn and jer_up, jer_dn.
Btw, I (just now) think we can apply these uncertainty evaluators in analyser codes. What we need for the evaluation are the informations of corresponding genJet in actual, and we are already keeping those informations in the analyser level. (So there is the evaluators in 'postprocess' codes!) I can move these evaluators into analyser level, if you want.

[1]

float fJESUp = 1, fJESDn = 1;
if ( !payloadName_.empty() ) {
jecUnc->setJetEta(jetEta);
jecUnc->setJetPt(jetPt); // here you must use the CORRECTED jet pt
fJESUp = 1 + jecUnc->getUncertainty(true);
jecUnc->setJetEta(jetEta);
jecUnc->setJetPt(jetPt); // here you must use the CORRECTED jet pt
fJESDn = 1 - jecUnc->getUncertainty(false);
}

[2]
float fJERUp = 1, fJERDn = 1;
if ( runOnMC_ ) {
// adding genJet
auto genJet = jet.genJetFwdRef();
JME::JetParameters jetPars = {{JME::Binning::JetPt, jetPt},
{JME::Binning::JetEta, jetEta},
{JME::Binning::Rho, rho}};
const double jetRes = jetResObj.getResolution(jetPars); // Note: this is relative resolution.
const double cJERUp = jetResSFObj.getScaleFactor(jetPars, Variation::UP);
const double cJERDn = jetResSFObj.getScaleFactor(jetPars, Variation::DOWN);
// JER - apply scaling method if matched genJet is found,
// apply gaussian smearing method if unmatched
if ( genJet.isNonnull() && deltaR(genJet->p4(), jet.p4()) < 0.2
&& std::abs(genJet->pt() - jetPt) < jetRes * 3 * jetPt )
{
const double genJetPt = genJet->pt();
const double dPt = jetPt - genJetPt;
fJERUp = std::max(0., (genJetPt + dPt * cJERUp) / jetPt);
fJERDn = std::max(0., (genJetPt + dPt * cJERDn) / jetPt);
} else {
const double smear = CLHEP::RandGaussQ::shoot(rng_);
fJERUp = ( cJERUp <= 1 ? 1 : 1 + smear * jetRes * sqrt(cJERUp * cJERUp - 1) );
fJERDn = ( cJERDn <= 1 ? 1 : 1 + smear * jetRes * sqrt(cJERDn * cJERDn - 1) );
}
}

[3] https://github.com/vallot/CATTools/blob/cat90x/CatProducer/plugins/CATJetProducer.cc#L218-L228
[4] https://github.com/vallot/CATTools/blob/cat90x/CatProducer/plugins/CATJetProducer.cc#L238-L266

@jshlee
Copy link
Collaborator

jshlee commented Nov 20, 2018

should be able to just do tlorentzvector*Jet_jes_up

@quark2
Copy link
Contributor Author

quark2 commented Nov 20, 2018

Yep, actually my instruction in the above comment is mathematically equivalent to yours.

Btw strangely, the postprocessing tool provided by nanoAOD guys gives evaluations for each of mass of jets and pt of jets separately (see [1] and [2] and [3]). I think it gives somewhat different factors on mass and pt, where other evaluations are applied to mass (JMS, JMR, see [2]). Also I think the nominal values of JMS and JMR for mass in their code are not unit in default. I think we need to check them, at least whether we can ignore them or not

[1] https://github.com/cms-nanoAOD/nanoAOD-tools/blob/master/python/postprocessing/modules/jme/jetmetUncertainties.py#L237-L259
[2] https://github.com/cms-nanoAOD/nanoAOD-tools/blob/master/python/postprocessing/modules/jme/jetmetUncertainties.py#L270-L283
[3] https://github.com/cms-nanoAOD/nanoAOD-tools/blob/master/python/postprocessing/modules/jme/jetmetUncertainties.py#L308-L319

@watson-ij
Copy link
Contributor

Yes, let's find the instructions recommended by the JetMET group, and the particular instructions on how we should evaluate the systematics from the Top group. My preference would be that if we can evaluate the systematics using the nano tuples only (i.e. don't need anything extra from MiniAOD collections) that we just add our own little analyser to do it (or adapt nanoAOD-tools or someone elses code or so), and be able to run on the fly.

But step 1 is to check the recommendations on the twiki.

@jshlee
Copy link
Collaborator

jshlee commented Nov 29, 2018

is this done?

@quark2
Copy link
Contributor Author

quark2 commented Nov 29, 2018

Nope... I've implemented the product code for this into on-the-fly level, and now I'm testing it.

@quark2
Copy link
Contributor Author

quark2 commented Nov 29, 2018

Also, as mentioned in the last comment in the related issue ([1]), I think we need to have a discussion about the additional jet mass factor. (I should bring this topic to the last nanoAOD meeting...)

[1] #57

@watson-ij
Copy link
Contributor

Weren't we trying to not have to use CMSSW?

@quark2
Copy link
Contributor Author

quark2 commented Dec 18, 2018

Ah, I just used DataFormats/Math/interface/deltaR.h ([1]). No more than this header file is needed for this change

[1] https://github.com/cms-sw/cmssw/blob/CMSSW_9_4_X/DataFormats/Math/interface/deltaR.h

@watson-ij
Copy link
Contributor

No more than the header file is needed, but it means we have to pull in all the CMSSW to use it, i.e. can't be run locally.

@quark2
Copy link
Contributor Author

quark2 commented Dec 18, 2018

Alright, then I'll roll back this.
But in our analyser code I already used CMSSW stuffs... (See [1]). These header files contain the central part of JER/JES study. What about them...? Should we duplicate them?

[1]

#include "CondFormats/JetMETObjects/interface/JetCorrectionUncertainty.h"
#include "CondFormats/JetMETObjects/interface/JetCorrectorParameters.h"
#include "JetMETCorrections/Objects/interface/JetCorrectionsRecord.h"
#include "JetMETCorrections/Modules/interface/JetResolution.h"

@quark2
Copy link
Contributor Author

quark2 commented Dec 18, 2018

Well, I've read codes for JER/JES in CMSSW I used. I think there dependency of CMSSW is almost trivial (even JER part is already designed with standalone case), so I can port them into our nano standalone, that is, into nano/external. Shall I do this?

@watson-ij
Copy link
Contributor

Yep, I would prefer it if its simple enough. So far, we're still able to compile and test analysis/ standalone, if its possible to keep this structure if we can. Thanks!

@watson-ij
Copy link
Contributor

But of course, we should document where we take it from so we can check the updates as the new calibrations come through.

@quark2
Copy link
Contributor Author

quark2 commented Dec 18, 2018

Don't worry, that's a basic option. ;) I'll port asap.

@quark2
Copy link
Contributor Author

quark2 commented Dec 18, 2018

Hm, seems like the JER files are not the problem, but there might be something unexpected. I'm looking for what the heck it is.

@quark2
Copy link
Contributor Author

quark2 commented Dec 19, 2018

I changed some namespace to avoid conflicts with CMSSW. This is tested in the CMSSW_9_4_6 docker image

@quark2
Copy link
Contributor Author

quark2 commented Dec 19, 2018

I checked that our nanoAOD producer only do JEC but not nominal JER smearing, and my code produces same result as 'the nominal JER smearing in CMSSW' (see [1]). See my slides ([2]) which summarizes all of this.

[1] https://github.com/cms-sw/cmssw/blob/CMSSW_9_4_X/PhysicsTools/PatUtils/interface/SmearedJetProducerT.h
[2] https://docs.google.com/presentation/d/1xMP5nUDKr3_H9tT7G05K_SGvQ3F-k7M1CEnqidk9oW0/edit?usp=sharing

@quark2
Copy link
Contributor Author

quark2 commented Dec 20, 2018

I changed some of my test code (the jetUncEval_compare branch); because I turned off the pre-cut on jets in nanoAOD production (see [1]), the size of jet arraries is not enough so that it causes memory corruption. (Don't worry, with the pre-cut, our current size seems enough.) So, only for this test, I changed the size (see [2]). (ONLY FOR THIS TEST BRANCH. Not for pull request at all.) This solves the issue on JEC discrepancy in full event test (see p. 20 in my slides).

But still the discrepancy on smearing is remaining. Hm...

[1] quark2/cmssw@820c63b
[2] quark2@512f53d

@quark2
Copy link
Contributor Author

quark2 commented Dec 25, 2018

I added a slide for comparison of jets before smearing with after-smearing ones. Does it make sense?

// In default, these are same as the original ones, but when a user wants to study systematic uncertainty
// so that he/she needs to switch them to the evaluated ones,
// just touching them in anlalyser class will be okay, and this is for it.
void topObjectSelection::GetJetMassPt(UInt_t nIdx,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't everyone need to do this? In which case, why are we doing it in top objects and not a level above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean, it would be better if this function (and the stuffs for initialization in the constructor) is in mother class, that is, nanoBase?

@@ -1,4 +1,6 @@
<use name="nano/external" />
<use name="JetMETCorrections/Objects"/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can remove this now

@@ -87,8 +92,9 @@ class topObjectSelection : public nanoBase

std::vector<TParticle> genJetSelection();

topObjectSelection(TTree *tree=0, TTree *had=0, TTree *hadTruth=0, Bool_t isMC = false);
topObjectSelection(TTree *tree=0, Bool_t isMC=false) : topObjectSelection(tree, 0, 0, isMC) {}
topObjectSelection(TTree *tree=0, TTree *had=0, TTree *hadTruth=0, Bool_t isMC = false, UInt_t unFlag = 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we name unFlag better? Its just for JES/JER, so maybe jetUnc?

const double jetRes = m_jetResObj.getResolution(jetPars); // Note: this is relative resolution.
const double cJER = m_jetResSFObj.getScaleFactor(jetPars,
((unFlag & (OptFlag_JER_Up | OptFlag_JER_Dn)) == 0 ? Variation::NOMINAL :
((unFlag & OptFlag_JER_Up) != 0 ? Variation::UP : Variation::DOWN)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit confusing, we can use plain flags rather than bit fields for these options

@tt8888tt tt8888tt added the Analyser-common Analyser related but change on common code (AFFECT TO OTHERS) label Mar 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Analyser-common Analyser related but change on common code (AFFECT TO OTHERS)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants