Izisopou patch miniaod by izisopou · Pull Request #29 · cms-jet/JetMETAnalysis

izisopou · 2020-10-28T17:15:54Z

Changes suggested by JERC conveners and implemented to run and produce ntuples from MINIAOD:

Replace genParticles with prunedGenParticles
Change the gen jet clustering to packedGenParticles
Switch fast jet producer to use packed PF candidates, both the target jets and the kt6 rho
replace PV with offlineSlimmedPrimaryVertices
Ditch the pfPileUpJME and pfNoPileUpJME - replace with process.PFCandCHS = cms.EDFilter("CandPtrSelector", src = cms.InputTag("packedPFCandidates"), cut = cms.string("fromPV() > 1")), and replace the relevant thing inside pfNoPileUpJME with PFCandCHS

A slightly modified script of the xrootd/XrdCl/XrdClFileSystem.hh one, in order to avoid compilation errors.

This change includes the XrdClFileSystem_v2.hh script instead of the XrdClFileSystem.hh so as to avoid compilation errors.

aperloff · 2020-11-03T16:53:17Z

Why did you copy an entire file from the XRootD code into this package, much less a header not in the interface folder? You're not going to want to maintain XrdClFileSystem_v2.hh. A better strategy is to put in static_asserts and header guards to make sure you pick up the correct files or exit right away upon a missing dependency.

aperloff · 2020-11-03T16:54:51Z

JetAnalyzers/interface/JetResponseAnalyzer.hh

  edm::EDGetTokenT<PFCandidateView> srcPFCandidates_;
  edm::EDGetTokenT<std::vector<edm::FwdPtr<reco::PFCandidate> > > srcPFCandidatesAsFwdPtr_;
-  edm::EDGetTokenT<vector<reco::GenParticle> > srcGenParticles_;
+//  edm::EDGetTokenT<vector<reco::GenParticle> > srcGenParticles_;


You should remove code, specifically commented code, which is no longer needed. If you need to switch back an forth you should consider using an `edm::View``` and then casting to the correct base type.

Okay, I removed the code that is no longer needed.

aperloff · 2020-11-03T17:01:45Z

JetAnalyzers/python/Defaults_cff.py

    saveCandidates  = cms.bool(False),
    # MATCHING MODE: deltaR(ref,jet)
-    deltaRMax       = cms.double(0.25),
+    deltaRMax       = cms.double(0.4),


I've been out of the loop for a while, so feel free to tell me I don't know the latest developments. That said, 0.4 seems awfully permissive for the deltaR matching. That's the entire cone radius for an R=0.4 jet. The default here should be the most often used value and should not change much. Is that why you changed this to 0.4.

Apologies for the confusion, this was actually a test version and we will fix it and upload the correct one with the values that were before. (However, this does not affect the results because we require dr max value of 0.2 in the MC Truth JECs derivation.)

aperloff · 2020-11-03T17:01:50Z

JetAnalyzers/python/Defaults_cff.py

    doflavor          = cms.bool(False),
    noabsflavors      = cms.bool(False),
-    drmax             = cms.double(0.3),
+    drmax             = cms.double(0.4),


I would imagine this should match the value in the JetResponseParameters, so keep that in mind when answering my previous comment.

aperloff · 2020-11-03T17:04:50Z

JetAnalyzers/python/JetReconstruction_cff.py

 #! PF JETS CHS
 #!
-ak1PFCHSJets    = ak1PFJets.clone( src = 'pfNoPileUpJME' )
+


Please remove these newlines to keep the style the same among the various sections of the code. You should strive to keep the formatting consistent throughout the code.

aperloff · 2020-11-03T17:06:11Z

JetAnalyzers/python/addAlgorithm.py

 from CommonTools.PileupAlgos.Puppi_cff import *
+from JetMETAnalysis.JetAnalyzers.customizePuppiTune_cff import *
+
+


Please keep this to 1 newline to maintain the style.

aperloff · 2020-11-03T17:12:55Z

JetAnalyzers/src/JetResponseAnalyzer.cc

 }


+


Please try to match the style for this entire section (the getMult function).

aperloff · 2020-11-03T17:22:31Z

JetAnalyzers/src/JetResponseAnalyzer.cc

+
+    switch (std::abs(pfCand->pdgId())) {
+
+      case 211: //PFCandidate::h:       // charged hadron


Don't use unnamed constants. Instead, you should define enums as in

JetMETAnalysis/JetUtilities/interface/JRAEvent.h

Line 37 in 9ea9753

enum Flavor{all=0, h, e, mu, gamma, h0, h_HF, egamma_HF, chs, numFlavors, X}; //X=undefined

and use those as your cases.

You also only really have two cases:

You add one to the nMult variable

You add one to the chMult variable
You should combine these cases using and OR of the conditions inside if, else if, else statements. This will reduce the number of comparisons. You can use the std::any_of function to reduce the verbosity of the conditional (https://www.cplusplus.com/reference/algorithm/any_of/).

aperloff · 2020-11-03T17:24:49Z

JetAnalyzers/src/JetResponseAnalyzer.cc

           JRAEvt_->jtmuf ->push_back(pfJetRef->muonEnergyFraction()         *JRAEvt_->jtjec->at(JRAEvt_->nref));
           JRAEvt_->jthfhf->push_back(pfJetRef->HFHadronEnergyFraction()     *JRAEvt_->jtjec->at(JRAEvt_->nref));
           JRAEvt_->jthfef->push_back(pfJetRef->HFEMEnergyFraction()         *JRAEvt_->jtjec->at(JRAEvt_->nref));
+int chMult=0, nMult=0;


Spacing issue

aperloff · 2020-11-03T17:25:16Z

JetUtilities/interface/JRAEvent.h

   vector<Int_t>*   refpdgid_physicsDef;
   vector<Float_t>* refe;
   vector<Float_t>* refpt;
+   vector<Int_t>* refnMult;


Match style

Same comment for all of the nMult and chMult lines.

aperloff · 2020-11-03T17:27:17Z

JetUtilities/src/JRAEvent.cc

   fChain->Branch("evt", &evt, "evt/L");
   fChain->Branch("nref", &nref, "nref/b");
   fChain->Branch("refrank", "vector<UChar_t>", &refrank);
+   fChain->Branch("refpdgid", "vector<Int_t>", &refpdgid);


This change means that the refpdgid branch will always be loaded. Is that want you want?

aperloff · 2020-11-03T17:29:35Z

You might also consider adding a GitHub action to run clang-tidy/clang-format on the code each time there is a PR or or push to the repo. This will help to keep the code in good shape and to help with formatting.

1st recipe for Run-3 PF-Hadron Calibrations

izisopou added 10 commits October 28, 2020 18:05

Update BuildFile.xml

8498413

Update JetResponseAnalyzer.hh

2c2decb

Update JetReconstruction_cff.py

5588ac5

Update addAlgorithm.py

2fd6c93

Update JetResponseAnalyzer.cc

77d7747

Update Defaults_cff.py

345364f

Update JRAEvent.cc

f9c3488

Update JRAEvent.h

650984b

Create XrdClFileSystem_v2.hh

8edaf1c

A slightly modified script of the xrootd/XrdCl/XrdClFileSystem.hh one, in order to avoid compilation errors.

Update jet_correction_analyzer_x.cc

19a622a

This change includes the XrdClFileSystem_v2.hh script instead of the XrdClFileSystem.hh so as to avoid compilation errors.

aperloff reviewed Nov 3, 2020

View reviewed changes

This was referenced Nov 3, 2020

Update Defaults_cff.py #28

Open

Izisopou patch new fitting approaches #30

Open

izisopou and others added 9 commits November 5, 2020 19:47

Delete XrdClFileSystem_v2.hh

a60f42d

Update jet_correction_analyzer_x.cc

b9712c6

Update JetResponseAnalyzer.hh

dbf79af

Update JetReconstruction_cff.py

a80a146

Update addAlgorithm.py

34aebad

Update JetResponseAnalyzer.cc

e1fb630

Update JRAEvent.h

28e40e4

Update Defaults_cff.py

95ad4db

Update HistogramUtilities.cc

c30dca5

sparedes pushed a commit to sparedes/JetMETAnalysis that referenced this pull request Dec 20, 2021

Merge pull request cms-jet#29 from missirol/run3_devel_111X

7e3fbad

1st recipe for Run-3 PF-Hadron Calibrations

		from CommonTools.PileupAlgos.Puppi_cff import *
		from JetMETAnalysis.JetAnalyzers.customizePuppiTune_cff import *


		switch (std::abs(pfCand->pdgId())) {

		case 211: //PFCandidate::h: // charged hadron

Comments

Conversation

izisopou commented Oct 28, 2020

Uh oh!

aperloff commented Nov 3, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

magdadiamant Nov 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aperloff commented Nov 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

magdadiamant Nov 21, 2020 •

edited

Loading