Fixing problems detected while testing

cms-sw · Dec 4, 2018 · 33be4ae · 33be4ae
1 parent 092316f
commit 33be4ae
Show file tree

Hide file tree

Showing 23 changed files with 168 additions and 168 deletions.
diff --git a/DQMOffline/Lumi/src/ElectronIdentifier.cc b/DQMOffline/Lumi/src/ElectronIdentifier.cc
@@ -157,7 +157,7 @@ bool ElectronIdentifier::passID(const reco::GsfElectronPtr& ele,edm::Handle<reco
    if( isolation(ele)/ele->pt()                         > cuts_[EleIDCutNames::ISO][ID_][region]) return false;
    if( std::abs(1.0 - ele->eSuperClusterOverP())/ele->ecalEnergy()  > cuts_[EleIDCutNames::ONEOVERE][ID_][region]) return false;
    if( (ele->gsfTrack()->hitPattern().numberOfAllHits(reco::HitPattern::MISSING_INNER_HITS)) > cuts_[EleIDCutNames::MISSINGHITS][ID_][region]) return false;
-   if( ConversionTools::hasMatchedConversion(*ele,conversions,beamspot->position())) return false;
+   if( ConversionTools::hasMatchedConversion(*ele,*conversions,beamspot->position())) return false;
 
    return true;
 }

diff --git a/DQMOffline/Trigger/plugins/LepHTMonitor.cc b/DQMOffline/Trigger/plugins/LepHTMonitor.cc
@@ -46,7 +46,7 @@ namespace{
     //Conversion matching
     bool pass_conversion = false;
     if (convs.isValid()) {
-      pass_conversion = !ConversionTools::hasMatchedConversion(el, convs, bs_position);
+      pass_conversion = !ConversionTools::hasMatchedConversion(el, *convs, bs_position);
     }
     else{
       edm::LogError("LepHTMonitor") << "Electron conversion matching failed.\n";

diff --git a/DQMServices/Examples/src/DQMExample_Step1.cc b/DQMServices/Examples/src/DQMExample_Step1.cc
@@ -395,7 +395,7 @@ bool DQMExample_Step1::MediumEle (const edm::Event & iEvent, const edm::EventSet
   iEvent.getByToken(theBSCollection_, BSHandle);
   const reco::BeamSpot BS = *BSHandle;
 
-  bool isConverted = ConversionTools::hasMatchedConversion(electron, conversions_h, BS.position());
+  bool isConverted = ConversionTools::hasMatchedConversion(electron, *conversions_h, BS.position());
 
   // default
   if(  (pt > 12.) && (fabs(eta) < 2.5) &&

diff --git a/EgammaAnalysis/ElectronTools/src/EGammaCutBasedEleId.cc b/EgammaAnalysis/ElectronTools/src/EGammaCutBasedEleId.cc
@@ -121,7 +121,7 @@ unsigned int EgammaCutBasedEleId::TestWP(WorkingPoint workingPoint,
     }
 
     // conversion rejection variables
-    bool vtxFitConversion = ConversionTools::hasMatchedConversion(ele, conversions, beamspot.position());
+    bool vtxFitConversion = ConversionTools::hasMatchedConversion(ele, *conversions, beamspot.position());
     float mHits = ele.gsfTrack()->hitPattern().numberOfLostHits(reco::HitPattern::MISSING_INNER_HITS); 
 
     // get the mask value

diff --git a/EgammaAnalysis/ElectronTools/test/ElectronIDValidationAnalyzer.cc b/EgammaAnalysis/ElectronTools/test/ElectronIDValidationAnalyzer.cc
@@ -231,8 +231,7 @@ ElectronIDValidationAnalyzer::analyze(const edm::Event& iEvent, const edm::Event
      expectedMissingInnerHits_ = el->gsfTrack()->hitPattern().numberOfLostHits(missingHitType);
      passConversionVeto_ = false;
      if( thebs.isValid() && convs.isValid() ) {
-       passConversionVeto_ = !ConversionTools::hasMatchedConversion(*el,convs,
-								   thebs->position());
+       passConversionVeto_ = !ConversionTools::hasMatchedConversion(*el,*convs,thebs->position());
      }else{
        std::cout << "\n\nERROR!!! conversions not found!!!\n" ;
      }

diff --git a/EgammaAnalysis/ElectronTools/test/MiniAODElectronIDValidationAnalyzer.cc b/EgammaAnalysis/ElectronTools/test/MiniAODElectronIDValidationAnalyzer.cc
@@ -242,8 +242,7 @@ MiniAODElectronIDValidationAnalyzer::analyze(const edm::Event& iEvent, const edm
     expectedMissingInnerHits_ = el->gsfTrack()->hitPattern().numberOfLostHits(missingHitType);
     passConversionVeto_ = false;
     if( thebs.isValid() && convs.isValid() ) {
-      passConversionVeto_ = !ConversionTools::hasMatchedConversion(*el,convs,
-								   thebs->position());
+      passConversionVeto_ = !ConversionTools::hasMatchedConversion(*el,*convs,thebs->position());
     }else{
       printf("\n\nERROR!!! conversions not found!!!\n");
     }

diff --git a/HLTriggerOffline/SUSYBSM/src/SUSY_HLT_SingleLepton.cc b/HLTriggerOffline/SUSYBSM/src/SUSY_HLT_SingleLepton.cc
@@ -67,7 +67,7 @@ namespace{
     bool pass_conversion = false;
     if(convs.isValid()){
       try{
-        pass_conversion = !ConversionTools::hasMatchedConversion(el, convs, bs_position);
+        pass_conversion = !ConversionTools::hasMatchedConversion(el, *convs, bs_position);
       }catch(...){
         edm::LogError("SUSY_HLT_SingleLepton") << "Electron conversion matching failed.\n";
         return false;

diff --git a/RecoBTag/SoftLepton/plugins/SoftPFElectronTagInfoProducer.cc b/RecoBTag/SoftLepton/plugins/SoftPFElectronTagInfoProducer.cc
@@ -85,7 +85,7 @@ void SoftPFElectronTagInfoProducer::produce(edm::Event& iEvent, const edm::Event
 				if(!patelec->passConversionVeto()) continue;
 			}
 			else{
-				if(ConversionTools::hasMatchedConversion(*(recoelectron),hConversions,beamspot.position())) continue;
+				if(ConversionTools::hasMatchedConversion(*(recoelectron),*hConversions,beamspot.position())) continue;
 			}
 			//Make sure that the electron is inside the jet
 			if(reco::deltaR2((*recoelectron),(*jetRef))>DeltaRElectronJet*DeltaRElectronJet) continue;

diff --git a/RecoEgamma/EgammaTools/interface/EffectiveAreas.h b/RecoEgamma/EgammaTools/interface/EffectiveAreas.h
@@ -10,7 +10,6 @@ class EffectiveAreas {
 public:
   // Constructor, destructor
   EffectiveAreas(const std::string& filename);
-  ~EffectiveAreas();
 
   // Accessors
   const float getEffectiveArea(float eta) const;

diff --git a/RecoEgamma/EgammaTools/interface/MVAVariableManager.h b/RecoEgamma/EgammaTools/interface/MVAVariableManager.h
@@ -57,13 +57,13 @@ class MVAVariableManager {
         return nVars_;
     }
 
-    float getValue(int index, const ParticleType* particle, const std::vector<float>& auxVariables) const {
+    float getValue(int index, const ParticleType& particle, const std::vector<float>& auxVariables) const {
         float value;
 
         MVAVariableInfo varInfo = variableInfos_[index];
 
         if (varInfo.auxIndex >= 0) value = auxVariables[varInfo.auxIndex];
-        else value = functions_[index](*particle);
+        else value = functions_[index](particle);
 
         if (varInfo.hasLowerClip && value < varInfo.lowerClipValue) {
             value = varInfo.lowerClipValue;

diff --git a/RecoEgamma/EgammaTools/src/ConversionTools.cc b/RecoEgamma/EgammaTools/src/ConversionTools.cc
@@ -1,4 +1,3 @@
-
 #include <TMath.h>
 #include "RecoEgamma/EgammaTools/interface/ConversionTools.h"
 #include "DataFormats/EgammaCandidates/interface/Conversion.h"
@@ -297,28 +296,8 @@ bool ConversionTools::hasMatchedPromptElectron(const reco::SuperClusterRef &sc,
                    const reco::ConversionCollection &convCol, const math::XYZPoint &beamspot, bool allowCkfMatch, float lxyMin, float probMin, unsigned int nHitsBeforeVtxMax)
 {
 
-  //check if a given SuperCluster matches to at least one GsfElectron having zero expected inner hits
-  //and not matching any conversion in the collection passing the quality cuts
-
-  if (sc.isNull()) return false;
-
-  for(auto const& it : eleCol) {
-    //match electron to supercluster
-    if (it.superCluster()!=sc) continue;
-
-    //check expected inner hits
-    if (it.gsfTrack()->hitPattern().numberOfLostHits(reco::HitPattern::MISSING_INNER_HITS) > 0) continue;
-
-    //check if electron is matching to a conversion
-    if (hasMatchedConversion(it,convCol,beamspot,allowCkfMatch,lxyMin,probMin,nHitsBeforeVtxMax)) continue;
-
-
-    return true;
-  }
-
-  return false;
-
-
+  return !(matchedPromptElectron(sc, eleCol, convCol, beamspot,
+              allowCkfMatch, lxyMin, probMin, nHitsBeforeVtxMax) == nullptr);
 }
 
 

diff --git a/RecoEgamma/EgammaTools/src/EffectiveAreas.cc b/RecoEgamma/EgammaTools/src/EffectiveAreas.cc
@@ -43,9 +43,6 @@ EffectiveAreas::EffectiveAreas(const std::string& filename):
   checkConsistency();
 }
 
-EffectiveAreas::~EffectiveAreas(){
-}
-
 // Return effective area for given eta
 const float EffectiveAreas::getEffectiveArea(float eta) const{
 

diff --git a/RecoEgamma/ElectronIdentification/interface/ElectronMVAEstimatorRun2.h b/RecoEgamma/ElectronIdentification/interface/ElectronMVAEstimatorRun2.h
@@ -72,7 +72,7 @@ class ElectronMVAEstimatorRun2 : public AnyMVAEstimatorRun2Base {
 
   void init(const std::vector<std::string> &weightFileNames);
 
-  int findCategory( const reco::GsfElectron* electron) const;
+  int findCategory(reco::GsfElectron const& electron) const;
 
   std::vector<ThreadSafeStringCut<StringCutObjectSelector<reco::GsfElectron>, reco::GsfElectron>> categoryFunctions_;
   std::vector<int> nVariables_;

diff --git a/RecoEgamma/ElectronIdentification/plugins/ElectronMVANtuplizer.cc b/RecoEgamma/ElectronIdentification/plugins/ElectronMVANtuplizer.cc
@@ -74,13 +74,6 @@ class ElectronMVANtuplizer : public edm::one::EDAnalyzer<edm::one::SharedResourc
 
       // ----------member data ---------------------------
 
-      // other
-      TTree* tree_;
-
-      MVAVariableManager<reco::GsfElectron> mvaVarMngr_;
-      std::vector<float> vars_;
-      int nVars_;
-
       //global variables
       int nEvent_, nRun_, nLumi_;
       int genNpu_;
@@ -102,10 +95,7 @@ class ElectronMVANtuplizer : public edm::one::EDAnalyzer<edm::one::SharedResourc
       bool eleIsEEDeeGap_;
       bool eleIsEERingGap_;
 
-      // to hold ID decisions and categories
-      std::vector<int> mvaPasses_;
-      std::vector<float> mvaValues_;
-      std::vector<int> mvaCats_;
+      int eleIndex_;
 
       // config
       const bool isMC_;
@@ -135,8 +125,21 @@ class ElectronMVANtuplizer : public edm::one::EDAnalyzer<edm::one::SharedResourc
       const MultiTokenT<std::vector<PileupSummaryInfo>> pileup_;
       const MultiTokenT<edm::View<reco::GenParticle>>   genParticles_;
 
+      // to hold ID decisions and categories
+      std::vector<int> mvaPasses_;
+      std::vector<float> mvaValues_;
+      std::vector<int> mvaCats_;
+
       // To get the auxiliary MVA variables
       const MVAVariableHelper<reco::GsfElectron> variableHelper_;
+
+      // other
+      TTree* tree_;
+
+      MVAVariableManager<reco::GsfElectron> mvaVarMngr_;
+      const int nVars_;
+      std::vector<float> vars_;
+
 };
 
 //
@@ -158,8 +161,7 @@ enum ElectronMatchType {
 // constructors and destructor
 //
 ElectronMVANtuplizer::ElectronMVANtuplizer(const edm::ParameterSet& iConfig)
-  : mvaVarMngr_            (iConfig.getParameter<std::string>("variableDefinition"))
-  , isMC_                  (iConfig.getParameter<bool>("isMC"))
+  : isMC_                  (iConfig.getParameter<bool>("isMC"))
   , deltaR_                (iConfig.getParameter<double>("deltaR"))
   , ptThreshold_           (iConfig.getParameter<double>("ptThreshold"))
   , eleMapTags_            (iConfig.getUntrackedParameter<std::vector<std::string>>("eleMVAs"))
@@ -175,40 +177,32 @@ ElectronMVANtuplizer::ElectronMVANtuplizer(const edm::ParameterSet& iConfig)
   , vertices_        (src_, consumesCollector(), iConfig, "vertices"    , "verticesMiniAOD")
   , pileup_          (src_, consumesCollector(), iConfig, "pileup"      , "pileupMiniAOD")
   , genParticles_    (src_, consumesCollector(), iConfig, "genParticles", "genParticlesMiniAOD")
-  , variableHelper_(consumesCollector())
+  , mvaPasses_             (nEleMaps_)
+  , mvaValues_             (nValMaps_)
+  , mvaCats_               (nCats_)
+  , variableHelper_        (consumesCollector())
+  , mvaVarMngr_            (iConfig.getParameter<std::string>("variableDefinition"))
+  , nVars_                 (mvaVarMngr_.getNVars())
+  , vars_                  (nVars_)
 {
     // eleMaps
-    for (size_t k = 0; k < nEleMaps_; ++k) {
-
-        eleMapTokens_.push_back(consumes<edm::ValueMap<bool> >(edm::InputTag(eleMapTags_[k])));
-
-        // Initialize vectors for holding ID decisions
-        mvaPasses_.push_back(0);
+    for (auto const& tag : eleMapTags_) {
+        eleMapTokens_.push_back(consumes<edm::ValueMap<bool> >(edm::InputTag(tag)));
     }
-
     // valMaps
-    for (size_t k = 0; k < nValMaps_; ++k) {
-        valMapTokens_.push_back(consumes<edm::ValueMap<float> >(edm::InputTag(valMapTags_[k])));
-
-        // Initialize vectors for holding MVA values
-        mvaValues_.push_back(0.0);
+    for (auto const& tag : valMapTags_) {
+        valMapTokens_.push_back(consumes<edm::ValueMap<float> >(edm::InputTag(tag)));
     }
-
     // categories
-    for (size_t k = 0; k < nCats_; ++k) {
-        mvaCatTokens_.push_back(consumes<edm::ValueMap<int> >(edm::InputTag(mvaCatTags_[k])));
-
-        // Initialize vectors for holding MVA values
-        mvaCats_.push_back(0);
+    for (auto const& tag : mvaCatTags_) {
+        mvaCatTokens_.push_back(consumes<edm::ValueMap<int> >(edm::InputTag(tag)));
     }
 
    // Book tree
    usesResource(TFileService::kSharedResource);
    edm::Service<TFileService> fs ;
    tree_  = fs->make<TTree>("tree","tree");
 
-   nVars_ = mvaVarMngr_.getNVars();
-
    tree_->Branch("nEvent",  &nEvent_);
    tree_->Branch("nRun",    &nRun_);
    tree_->Branch("nLumi",   &nLumi_);
@@ -222,10 +216,6 @@ ElectronMVANtuplizer::ElectronMVANtuplizer(const edm::ParameterSet& iConfig)
        tree_->Branch("matchedToGenEle",   &matchedToGenEle_);
    }
 
-   // Has to be in two different loops
-   for (int i = 0; i < nVars_; ++i) {
-       vars_.push_back(0.0);
-   }
    for (int i = 0; i < nVars_; ++i) {
        tree_->Branch(mvaVarMngr_.getName(i).c_str(), &vars_[i]);
    }
@@ -238,6 +228,8 @@ ElectronMVANtuplizer::ElectronMVANtuplizer(const edm::ParameterSet& iConfig)
    tree_->Branch("ele_isEEDeeGap",&eleIsEEDeeGap_);
    tree_->Branch("ele_isEERingGap",&eleIsEERingGap_);
 
+   tree_->Branch("ele_index",&eleIndex_);
+
    // IDs
    for (size_t k = 0; k < nValMaps_; ++k) {
        tree_->Branch(valMapBranchNames_[k].c_str() ,  &mvaValues_[k]);
@@ -316,8 +308,11 @@ ElectronMVANtuplizer::analyze(const edm::Event& iEvent, const edm::EventSetup& i
         iEvent.getByToken(mvaCatTokens_[k],mvaCats[k]);
     }
 
+    eleIndex_ = -1;
     for(size_t iEle = 0; iEle < src->size(); ++iEle) {
 
+        ++eleIndex_;
+
         const auto ele =  src->ptrAt(iEle);
 
         eleQ_ = ele->charge();
@@ -329,7 +324,7 @@ ElectronMVANtuplizer::analyze(const edm::Event& iEvent, const edm::EventSetup& i
 
         for (int iVar = 0; iVar < nVars_; ++iVar) {
             std::vector<float> extraVariables = variableHelper_.getAuxVariables(ele, iEvent);
-            vars_[iVar] = mvaVarMngr_.getValue(iVar, &(*ele), extraVariables);
+            vars_[iVar] = mvaVarMngr_.getValue(iVar, *ele, extraVariables);
         }
 
         if (isMC_) {

diff --git a/RecoEgamma/ElectronIdentification/python/ElectronMVAValueMapProducer_cfi.py b/RecoEgamma/ElectronIdentification/python/ElectronMVAValueMapProducer_cfi.py
@@ -27,9 +27,6 @@
     import mvaEleID_Fall17_iso_V2_producer_config
 mvaConfigsForEleProducer.append( mvaEleID_Fall17_iso_V2_producer_config )
 
-# The producer to compute the MVA input variables which are not accessible with the cut parser
-from RecoEgamma.ElectronIdentification.electronMVAVariableHelper_cfi import *
-
 electronMVAValueMapProducer = cms.EDProducer('ElectronMVAValueMapProducer',
                                              # The module automatically detects AOD vs miniAOD, so we configure both
                                              #

diff --git a/RecoEgamma/ElectronIdentification/python/FWLite.py b/RecoEgamma/ElectronIdentification/python/FWLite.py
@@ -33,12 +33,17 @@ def __call__(self, ele, convs, beam_spot, rho, debug=False):
 
 # Import information needed to construct the e/gamma MVAs
 
-from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_tools import EleMVA_6CategoriesCuts, mvaVariablesFile, EleMVA_3CategoriesCuts
-
-from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Fall17_iso_V2_cff import mvaWeightFiles as Fall17_iso_V2_weightFiles
-from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Fall17_noIso_V2_cff import mvaWeightFiles as Fall17_noIso_V2_weightFiles
-from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Spring16_GeneralPurpose_V1_cff import mvaSpring16WeightFiles_V1 as mvaSpring16GPWeightFiles_V1
-from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Spring16_HZZ_V1_cff import mvaSpring16WeightFiles_V1 as mvaSpring16HZZWeightFiles_V1
+from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_tools \
+        import EleMVA_6CategoriesCuts, mvaVariablesFile, EleMVA_3CategoriesCuts
+
+from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Fall17_iso_V2_cff \
+        import mvaWeightFiles as Fall17_iso_V2_weightFiles
+from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Fall17_noIso_V2_cff \
+        import mvaWeightFiles as Fall17_noIso_V2_weightFiles
+from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Spring16_GeneralPurpose_V1_cff \
+        import mvaSpring16WeightFiles_V1 as mvaSpring16GPWeightFiles_V1
+from RecoEgamma.ElectronIdentification.Identification.mvaElectronID_Spring16_HZZ_V1_cff \
+        import mvaSpring16WeightFiles_V1 as mvaSpring16HZZWeightFiles_V1
 
 # Dictionary with the relecant e/gmma MVAs
 
@@ -49,6 +54,6 @@ def __call__(self, ele, convs, beam_spot, rho, debug=False):
                                       EleMVA_6CategoriesCuts, Fall17_noIso_V2_weightFiles, mvaVariablesFile),
         "Spring16HZZV1" : ElectronMVAID("ElectronMVAEstimatorRun2","Spring16HZZV1",
                                       EleMVA_6CategoriesCuts, mvaSpring16HZZWeightFiles_V1, mvaVariablesFile),
-        "Spring16V1"    : ElectronMVAID("ElectronMVAEstimatorRun2","Spring16GeneralPurposeV1",
+        "Spring16GPV1"  : ElectronMVAID("ElectronMVAEstimatorRun2","Spring16GeneralPurposeV1",
                                       EleMVA_3CategoriesCuts, mvaSpring16GPWeightFiles_V1, mvaVariablesFile),
 }
diff --git a/RecoEgamma/ElectronIdentification/src/ElectronMVAEstimatorRun2.cc b/RecoEgamma/ElectronIdentification/src/ElectronMVAEstimatorRun2.cc
@@ -39,8 +39,6 @@ ElectronMVAEstimatorRun2::ElectronMVAEstimatorRun2( const std::string& mvaTag,
   init(weightFileNames);
 }
 
-
-
 void ElectronMVAEstimatorRun2::init(const std::vector<std::string> &weightFileNames) {
 
   if(isDebug()) {
@@ -105,7 +103,7 @@ mvaValue( const reco::Candidate* candidate, const std::vector<float>& auxVariabl
   std::vector<float> vars;
 
   for (int i = 0; i < nVariables_[iCategory]; ++i) {
-      vars.push_back(mvaVarMngr_.getValue(variables_[iCategory][i], electron, auxVariables));
+      vars.push_back(mvaVarMngr_.getValue(variables_[iCategory][i], *electron, auxVariables));
   }
 
   if(isDebug()) {
@@ -134,19 +132,19 @@ int ElectronMVAEstimatorRun2::findCategory( const reco::Candidate* candidate) co
       << " but appears to be neither" << std::endl;
   }
 
-  return findCategory(electron);
+  return findCategory(*electron);
 
 }
 
-int ElectronMVAEstimatorRun2::findCategory( const reco::GsfElectron* electron) const {
+int ElectronMVAEstimatorRun2::findCategory(reco::GsfElectron const& electron) const {
 
   for (int i = 0; i < getNCategories(); ++i) {
-      if (categoryFunctions_[i](*electron)) return i;
+      if (categoryFunctions_[i](electron)) return i;
   }
 
   edm::LogWarning  ("MVA warning") <<
-      "category not defined for particle with pt " << electron->pt() << " GeV, eta " <<
-          electron->superCluster()->eta() << " in ElectronMVAEstimatorRun2" << getTag();
+      "category not defined for particle with pt " << electron.pt() << " GeV, eta " <<
+          electron.superCluster()->eta() << " in ElectronMVAEstimatorRun2" << getTag();
 
   return -1;