From b7d7959e7c60775f63e13acb56169da4b71b271b Mon Sep 17 00:00:00 2001 From: nancy Date: Mon, 29 Jul 2019 12:21:41 +0200 Subject: [PATCH 1/6] Final (?) fix to the chi2 problem. In the end the 'source' of the different chi2 was residing in the old recovery code which has never been turned on hence the problem never spot. With this commit the recovery with the new BDT is still on. Will be switched off with the next one --- .../BuildFile.xml | 1 + .../interface/EcalDeadChannelRecoveryAlgos.h | 21 ++- .../interface/EcalDeadChannelRecoveryBDTG.h | 52 ++++++ .../src/EcalDeadChannelRecoveryAlgos.cc | 33 ++-- .../src/EcalDeadChannelRecoveryBDTG.cc | 150 ++++++++++++++++++ .../EcalDeadChannelRecoveryProducers.cc | 10 +- .../plugins/EcalRecHitProducer.cc | 7 + .../plugins/EcalRecHitWorkerRecover.cc | 18 ++- .../plugins/EcalRecHitWorkerRecover.h | 1 + .../EcalRecProducers/python/ecalRecHit_cfi.py | 13 +- 10 files changed, 275 insertions(+), 31 deletions(-) create mode 100644 RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h create mode 100644 RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/BuildFile.xml b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/BuildFile.xml index 72df18a54ebf1..eab37ca28a8e6 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/BuildFile.xml +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/BuildFile.xml @@ -3,6 +3,7 @@ + diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h index 1b069b869fff7..6f2ec2746f05e 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h @@ -7,16 +7,25 @@ #include "DataFormats/EcalDetId/interface/EBDetId.h" #include -#include "RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryNN.h" +#include "FWCore/ParameterSet/interface/ParameterSetDescription.h" + +#include "RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h" + +#include "FWCore/ParameterSet/interface/ParameterSet.h" template class EcalDeadChannelRecoveryAlgos { public: - void setCaloTopology(const CaloTopology *topology); - EcalRecHit correct( - const DetIdT id, const EcalRecHitCollection &hit_collection, std::string algo, double Sum8Cut, bool *AccFlag); + void setParameters(const edm::ParameterSet &ps); + void setCaloTopology(std::string algo, const CaloTopology *topology); + float correct(const DetIdT id, + const EcalRecHitCollection &hit_collection, + std::string algo, + double single8Cut, + double sum8Cut, + bool *accFlag); private: - EcalDeadChannelRecoveryNN nn; + EcalDeadChannelRecoveryBDTG bdtg_; }; -#endif // RecoLocalCalo_EcalDeadChannelRecoveryAlgos_EcalDeadChannelRecoveryAlgos_HH +#endif diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h new file mode 100644 index 0000000000000..58faf836b6594 --- /dev/null +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h @@ -0,0 +1,52 @@ +#ifndef RecoLocalCalo_EcalDeadChannelRecoveryAlgos_EcalDeadChannelRecoveryBDTG_H +#define RecoLocalCalo_EcalDeadChannelRecoveryAlgos_EcalDeadChannelRecoveryBDTG_H + +#include "DataFormats/EcalRecHit/interface/EcalRecHit.h" +#include "DataFormats/EcalRecHit/interface/EcalRecHitCollections.h" + +#include "DataFormats/EcalDetId/interface/EBDetId.h" +#include "DataFormats/EcalDetId/interface/EEDetId.h" + +#include "Geometry/CaloTopology/interface/CaloTopology.h" +#include "Geometry/CaloTopology/interface/CaloSubdetectorTopology.h" + +#include "CommonTools/MVAUtils/interface/TMVAZipReader.h" +#include "FWCore/ParameterSet/interface/ParameterSet.h" + +#include "TMVA/Reader.h" + +#include +#include + +template +class EcalDeadChannelRecoveryBDTG { +public: + EcalDeadChannelRecoveryBDTG(); + ~EcalDeadChannelRecoveryBDTG(); + + void setParameters(const edm::ParameterSet &ps); + void setCaloTopology(const CaloTopology *topo) { topology_ = topo; }; + + double recover( + const DetIdT id, const EcalRecHitCollection &hit_collection, double single8Cut, double sum8Cut, bool *acceptFlag); + + void loadFile(); + void addVariables(TMVA::Reader *reader); + +private: + const CaloTopology *topology_; + struct XtalMatrix { + std::array rEn, ieta, iphi; + float sumE8; + }; + + XtalMatrix mx_; + + edm::FileInPath bdtWeightFileNoCracks_; + edm::FileInPath bdtWeightFileCracks_; + + TMVA::Reader *readerNoCrack; + TMVA::Reader *readerCrack; +}; + +#endif diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc index 7306f41c43e91..3f1e3f40b21ce 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc @@ -8,30 +8,41 @@ // Feb 14 2013: Implementation of the criterion to select the "correct" // max. cont. crystal. // +//modified by S.Taroni, N. Marinelli 11 June 2019 #include "RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h" #include "FWCore/MessageLogger/interface/MessageLogger.h" template -void EcalDeadChannelRecoveryAlgos::setCaloTopology(const CaloTopology *topo) { - nn.setCaloTopology(topo); +void EcalDeadChannelRecoveryAlgos::setParameters(const edm::ParameterSet &ps) { + bdtg_.setParameters(ps); } template -EcalRecHit EcalDeadChannelRecoveryAlgos::correct( - const T id, const EcalRecHitCollection &hit_collection, std::string algo, double Sum8Cut, bool *AcceptFlag) { - // recover as single dead channel - double NewEnergy = 0.0; +void EcalDeadChannelRecoveryAlgos::setCaloTopology(std::string algo, const CaloTopology *topo) { + bdtg_.setCaloTopology(topo); +} - if (algo == "NeuralNetworks") { - NewEnergy = this->nn.recover(id, hit_collection, Sum8Cut, AcceptFlag); +template +float EcalDeadChannelRecoveryAlgos::correct(const T id, + const EcalRecHitCollection &hit_collection, + std::string algo, + double single8Cut, + double sum8Cut, + bool *acceptFlag) { + // recover as single dead channel + double newEnergy = 0.0; + if (algo == "BDTG") { + *acceptFlag = false; + newEnergy = this->bdtg_.recover(id, hit_collection, single8Cut, sum8Cut, acceptFlag); //ADD here + if (newEnergy > 0.) + *acceptFlag = true; //bdtg set to 0 if there is more than one channel in the matrix that is not reponding } else { edm::LogError("EcalDeadChannelRecoveryAlgos") << "Invalid algorithm for dead channel recovery."; - *AcceptFlag = false; + *acceptFlag = false; } - uint32_t flag = 0; - return EcalRecHit(id, NewEnergy, 0, flag); + return newEnergy; } template class EcalDeadChannelRecoveryAlgos; diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc new file mode 100644 index 0000000000000..4a194135fcee3 --- /dev/null +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc @@ -0,0 +1,150 @@ +// Original Authors: S. Taroni, N. Marinelli +// University of Notre Dame - US +// Created: +// +// +// + +#include "RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h" +#include "RecoEcal/EgammaCoreTools/interface/EcalClusterTools.h" // can I use a egammatools here? +#include "FWCore/ParameterSet/interface/FileInPath.h" +#include + +#include +#include +#include +#include + +template <> +void EcalDeadChannelRecoveryBDTG::addVariables(TMVA::Reader *reader) { + for (int i = 0; i < 9; ++i) { + if (i == 4) + continue; + reader->AddVariable("E" + std::to_string(i + 1) + "/(E1+E2+E3+E4+E6+E7+E8+E9)", &(mx_.rEn[i])); + } + reader->AddVariable("E1+E2+E3+E4+E6+E7+E8+E9", &(mx_.sumE8)); + for (int i = 0; i < 9; ++i) + reader->AddVariable("iEta" + std::to_string(i + 1), &(mx_.ieta[i])); + for (int i = 0; i < 9; ++i) + reader->AddVariable("iPhi" + std::to_string(i + 1), &(mx_.iphi[i])); +} +template <> +void EcalDeadChannelRecoveryBDTG::loadFile() { + readerNoCrack = new TMVA::Reader("!Color:!Silent"); + readerCrack = new TMVA::Reader("!Color:!Silent"); + + this->addVariables(readerNoCrack); + this->addVariables(readerCrack); + + reco::details::loadTMVAWeights(readerNoCrack, "BDTG", bdtWeightFileNoCracks_.fullPath()); + reco::details::loadTMVAWeights(readerCrack, "BDTG", bdtWeightFileCracks_.fullPath()); +} + +template +EcalDeadChannelRecoveryBDTG::EcalDeadChannelRecoveryBDTG() {} + +template +EcalDeadChannelRecoveryBDTG::~EcalDeadChannelRecoveryBDTG() {} + +template <> +void EcalDeadChannelRecoveryBDTG::setParameters(const edm::ParameterSet &ps) { + bdtWeightFileNoCracks_ = ps.getParameter("bdtWeightFileNoCracks"); + bdtWeightFileCracks_ = ps.getParameter("bdtWeightFileCracks"); + + this->loadFile(); +} + +template <> +void EcalDeadChannelRecoveryBDTG::setParameters(const edm::ParameterSet &ps) {} + +template <> +double EcalDeadChannelRecoveryBDTG::recover( + const EEDetId id, const EcalRecHitCollection &hit_collection, double single8Cut, double sum8Cut, bool *acceptFlag) { + return 0; +} + +template <> +double EcalDeadChannelRecoveryBDTG::recover( + const EBDetId id, const EcalRecHitCollection &hit_collection, double single8Cut, double sum8Cut, bool *acceptFlag) { + bool isCrack = false; + int cellIndex = 0.; + double neighTotEn = 0.; + float val = 0.; + + //find the matrix around id + std::vector m3x3aroundDC = EcalClusterTools::matrixDetId(topology_, id, 1); + if (m3x3aroundDC.size() < 9) { + *acceptFlag = false; + return 0; + } + + // Loop over all cells in the vector "NxNaroundDC", and for each cell find it's energy + // (from the EcalRecHits collection). + for (auto const &theCells : m3x3aroundDC) { + EBDetId cell = EBDetId(theCells); + if (cell == id) { + + int iEtaCentral = std::abs(cell.ieta()); + int iPhiCentral = cell.iphi(); + + if (iEtaCentral < 2 || std::abs(iEtaCentral - 25) < 2 || std::abs(iEtaCentral - 45) < 2 || + std::abs(iEtaCentral - 65) < 2 || iEtaCentral > 83 || (int(iPhiCentral + 0.5) % 20 == 0)) + isCrack = true; + } + if (!cell.null()) { + EcalRecHitCollection::const_iterator goS_it = hit_collection.find(cell); + if (goS_it != hit_collection.end() && cell!=id) { + if (goS_it->energy() < single8Cut) { + *acceptFlag = false; + return 0.; + } else { + neighTotEn += goS_it->energy(); + mx_.rEn[cellIndex] = goS_it->energy(); + mx_.iphi[cellIndex] = cell.iphi(); + mx_.ieta[cellIndex] = cell.ieta(); + cellIndex++; + } + } else if (cell==id) { // the cell is the central one + mx_.rEn[cellIndex] = 0; + cellIndex++; + }else { //goS_it is not in the rechitcollection + *acceptFlag = false; + return 0.; + } + } else { //cell is null + *acceptFlag = false; + return 0.; + } + } + if (cellIndex > 0 && neighTotEn >= single8Cut * 8. && neighTotEn >= sum8Cut) { + bool allneighs = true; + mx_.sumE8 = neighTotEn; + for (unsigned int icell = 0; icell < 9; icell++) { + if (mx_.rEn[icell] < single8Cut && icell != 4) { + allneighs = false; + } + mx_.rEn[icell] = mx_.rEn[icell] / neighTotEn; + } + if (allneighs == true) { + // evaluate the regression + if (isCrack) { + val = exp((readerCrack->EvaluateRegression("BDTG"))[0]); + } else { + val = exp((readerNoCrack->EvaluateRegression("BDTG"))[0]); + } + + *acceptFlag = true; + //return the estimated energy + return val; + } else { + *acceptFlag = false; + return 0; + } + } else { + *acceptFlag = false; + return 0; + } +} + +template class EcalDeadChannelRecoveryBDTG; +template class EcalDeadChannelRecoveryBDTG; //not used. diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc index 5c19bac396782..47bcbd312779e 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc @@ -67,7 +67,8 @@ void EcalDeadChannelRecoveryProducers::produce(edm::Event& evt, const ed // create a unique_ptr to a EcalRecHitCollection, copy the RecHits into it and // put it in the Event: auto redCollection = std::make_unique(); - deadChannelCorrector.setCaloTopology(theCaloTopology.product()); + std::string dummy; + deadChannelCorrector.setCaloTopology(dummy, theCaloTopology.product()); // // Double loop over EcalRecHit collection and "dead" cell RecHits. @@ -80,9 +81,10 @@ void EcalDeadChannelRecoveryProducers::produce(edm::Event& evt, const ed if (it->detid() == *CheckDead) { OverADeadRecHit = true; bool AcceptRecHit = true; - EcalRecHit hit = deadChannelCorrector.correct( - it->detid(), *hit_collection, CorrectionMethod_, Sum8GeVThreshold_, &AcceptRecHit); - + float dummy = 0; + float ebEn = deadChannelCorrector.correct( + it->detid(), *hit_collection, CorrectionMethod_, Sum8GeVThreshold_, dummy, &AcceptRecHit); + EcalRecHit hit(it->detid(), ebEn, 0., EcalRecHit::kDead); if (hit.energy() != 0 and AcceptRecHit == true) { hit.setFlag(EcalRecHit::kNeighboursRecovered); } else { diff --git a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitProducer.cc b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitProducer.cc index 5a4eb79354303..0bf9119382d63 100644 --- a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitProducer.cc +++ b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitProducer.cc @@ -308,6 +308,13 @@ void EcalRecHitProducer::fillDescriptions(edm::ConfigurationDescriptions& descri desc.add("eeFEToBeRecovered", edm::InputTag("ecalDetIdToBeRecovered", "eeFE")); desc.add("ebDetIdToBeRecovered", edm::InputTag("ecalDetIdToBeRecovered", "ebDetId")); desc.add("singleChannelRecoveryThreshold", 8); + desc.add("sum8ChannelRecoveryThreshold", 0.); + desc.add("bdtWeightFileNoCracks", + edm::FileInPath("RecoLocalCalo/EcalDeadChannelRecoveryAlgos/data/BDTWeights/" + "bdtgAllRH_8GT700MeV_noCracks_ZskimData2017_v1.xml")); + desc.add("bdtWeightFileCracks", + edm::FileInPath("RecoLocalCalo/EcalDeadChannelRecoveryAlgos/data/BDTWeights/" + "bdtgAllRH_8GT700MeV_onlyCracks_ZskimData2017_v1.xml")); { std::vector temp1; temp1.reserve(9); diff --git a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc index 4ebbddd47ff3a..35cd1cf323907 100644 --- a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc +++ b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc @@ -24,6 +24,7 @@ EcalRecHitWorkerRecover::EcalRecHitWorkerRecover(const edm::ParameterSet& ps, ed // isolated channel recovery singleRecoveryMethod_ = ps.getParameter("singleChannelRecoveryMethod"); singleRecoveryThreshold_ = ps.getParameter("singleChannelRecoveryThreshold"); + sum8RecoveryThreshold_ = ps.getParameter("sum8ChannelRecoveryThreshold"); killDeadChannels_ = ps.getParameter("killDeadChannels"); recoverEBIsolatedChannels_ = ps.getParameter("recoverEBIsolatedChannels"); recoverEEIsolatedChannels_ = ps.getParameter("recoverEEIsolatedChannels"); @@ -40,6 +41,9 @@ EcalRecHitWorkerRecover::EcalRecHitWorkerRecover(const edm::ParameterSet& ps, ed tpDigiToken_ = c.consumes(ps.getParameter("triggerPrimitiveDigiCollection")); + + if (recoverEBIsolatedChannels_ && singleRecoveryMethod_ == "BDTG") + ebDeadChannelCorrector.setParameters(ps); } void EcalRecHitWorkerRecover::set(const edm::EventSetup& es) { @@ -120,12 +124,13 @@ bool EcalRecHitWorkerRecover::run(const edm::Event& evt, if (flags == EcalRecHitWorkerRecover::EB_single) { // recover as single dead channel - ebDeadChannelCorrector.setCaloTopology(caloTopology_.product()); + ebDeadChannelCorrector.setCaloTopology(singleRecoveryMethod_, caloTopology_.product()); // channel recovery. Accepted new RecHit has the flag AcceptRecHit=TRUE bool AcceptRecHit = true; - EcalRecHit hit = - ebDeadChannelCorrector.correct(detId, result, singleRecoveryMethod_, singleRecoveryThreshold_, &AcceptRecHit); + float ebEn = ebDeadChannelCorrector.correct( + detId, result, singleRecoveryMethod_, singleRecoveryThreshold_, sum8RecoveryThreshold_, &AcceptRecHit); + EcalRecHit hit(detId, ebEn, 0., EcalRecHit::kDead); if (hit.energy() != 0 and AcceptRecHit == true) { hit.setFlag(EcalRecHit::kNeighboursRecovered); @@ -137,12 +142,13 @@ bool EcalRecHitWorkerRecover::run(const edm::Event& evt, } else if (flags == EcalRecHitWorkerRecover::EE_single) { // recover as single dead channel - eeDeadChannelCorrector.setCaloTopology(caloTopology_.product()); + eeDeadChannelCorrector.setCaloTopology(singleRecoveryMethod_, caloTopology_.product()); // channel recovery. Accepted new RecHit has the flag AcceptRecHit=TRUE bool AcceptRecHit = true; - EcalRecHit hit = - eeDeadChannelCorrector.correct(detId, result, singleRecoveryMethod_, singleRecoveryThreshold_, &AcceptRecHit); + float eeEn = eeDeadChannelCorrector.correct( + detId, result, singleRecoveryMethod_, singleRecoveryThreshold_, sum8RecoveryThreshold_, &AcceptRecHit); + EcalRecHit hit(detId, eeEn, 0., EcalRecHit::kDead); if (hit.energy() != 0 and AcceptRecHit == true) { hit.setFlag(EcalRecHit::kNeighboursRecovered); } else { diff --git a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.h b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.h index 91eed40e1c109..a73d209792f7e 100644 --- a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.h +++ b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.h @@ -50,6 +50,7 @@ class EcalRecHitWorkerRecover : public EcalRecHitWorkerBaseClass { edm::ESHandle chStatus_; double singleRecoveryThreshold_; + double sum8RecoveryThreshold_; std::string singleRecoveryMethod_; bool killDeadChannels_; diff --git a/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py b/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py index 21e91ceadfe0c..cab0a08e6ad4a 100644 --- a/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py +++ b/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py @@ -49,7 +49,7 @@ # for channel recovery algoRecover = cms.string("EcalRecHitWorkerRecover"), - recoverEBIsolatedChannels = cms.bool(False), + recoverEBIsolatedChannels = cms.bool(True),##default is false recoverEEIsolatedChannels = cms.bool(False), recoverEBVFE = cms.bool(False), recoverEEVFE = cms.bool(False), @@ -76,8 +76,11 @@ eeDetIdToBeRecovered = cms.InputTag("ecalDetIdToBeRecovered:eeDetId"), ebFEToBeRecovered = cms.InputTag("ecalDetIdToBeRecovered:ebFE"), eeFEToBeRecovered = cms.InputTag("ecalDetIdToBeRecovered:eeFE"), - singleChannelRecoveryMethod = cms.string("NeuralNetworks"), - singleChannelRecoveryThreshold = cms.double(8), + singleChannelRecoveryMethod = cms.string("BDTG"), + singleChannelRecoveryThreshold = cms.double(0.70), #Threshold in GeV + sum8ChannelRecoveryThreshold = cms.double(0.), #Threshold in GeV + bdtWeightFileNoCracks = cms.FileInPath("RecoLocalCalo/EcalDeadChannelRecoveryAlgos/data/BDTWeights/bdtgAllRH_8GT700MeV_noCracks_ZskimData2017_v1.xml"), + bdtWeightFileCracks = cms.FileInPath("RecoLocalCalo/EcalDeadChannelRecoveryAlgos/data/BDTWeights/bdtgAllRH_8GT700MeV_onlyCracks_ZskimData2017_v1.xml"), triggerPrimitiveDigiCollection = cms.InputTag("ecalDigis:EcalTriggerPrimitives"), cleaningConfig=cleaningAlgoConfig, @@ -88,4 +91,6 @@ fastSim.toModify(ecalRecHit, killDeadChannels = False, recoverEBFE = False, - recoverEEFE = False) + recoverEEFE = False, + recoverEBIsolatedChannels = False + ) From 58ca071f75c22aae17818363c60797775ac889ba Mon Sep 17 00:00:00 2001 From: nancy Date: Mon, 29 Jul 2019 12:36:25 +0200 Subject: [PATCH 2/6] Code formatting --- .../src/EcalDeadChannelRecoveryBDTG.cc | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc index 4a194135fcee3..feec3461877f7 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc @@ -83,7 +83,6 @@ double EcalDeadChannelRecoveryBDTG::recover( for (auto const &theCells : m3x3aroundDC) { EBDetId cell = EBDetId(theCells); if (cell == id) { - int iEtaCentral = std::abs(cell.ieta()); int iPhiCentral = cell.iphi(); @@ -93,21 +92,21 @@ double EcalDeadChannelRecoveryBDTG::recover( } if (!cell.null()) { EcalRecHitCollection::const_iterator goS_it = hit_collection.find(cell); - if (goS_it != hit_collection.end() && cell!=id) { - if (goS_it->energy() < single8Cut) { - *acceptFlag = false; - return 0.; - } else { - neighTotEn += goS_it->energy(); - mx_.rEn[cellIndex] = goS_it->energy(); - mx_.iphi[cellIndex] = cell.iphi(); - mx_.ieta[cellIndex] = cell.ieta(); - cellIndex++; - } - } else if (cell==id) { // the cell is the central one - mx_.rEn[cellIndex] = 0; - cellIndex++; - }else { //goS_it is not in the rechitcollection + if (goS_it != hit_collection.end() && cell != id) { + if (goS_it->energy() < single8Cut) { + *acceptFlag = false; + return 0.; + } else { + neighTotEn += goS_it->energy(); + mx_.rEn[cellIndex] = goS_it->energy(); + mx_.iphi[cellIndex] = cell.iphi(); + mx_.ieta[cellIndex] = cell.ieta(); + cellIndex++; + } + } else if (cell == id) { // the cell is the central one + mx_.rEn[cellIndex] = 0; + cellIndex++; + } else { //goS_it is not in the rechitcollection *acceptFlag = false; return 0.; } From da9d251ac042c607e698a5c7135626377466c05c Mon Sep 17 00:00:00 2001 From: nancy Date: Tue, 30 Jul 2019 14:10:36 +0200 Subject: [PATCH 3/6] And now we switch OFF the channel recovery until DPG request ot switching on for large scale tests --- RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py b/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py index cab0a08e6ad4a..3f3b8c72641d5 100644 --- a/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py +++ b/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py @@ -49,7 +49,7 @@ # for channel recovery algoRecover = cms.string("EcalRecHitWorkerRecover"), - recoverEBIsolatedChannels = cms.bool(True),##default is false + recoverEBIsolatedChannels = cms.bool(False),##default is false recoverEEIsolatedChannels = cms.bool(False), recoverEBVFE = cms.bool(False), recoverEEVFE = cms.bool(False), From ec9e1027997906eac5b4c83541766ea9df5bb345 Mon Sep 17 00:00:00 2001 From: nancy Date: Wed, 31 Jul 2019 14:01:48 +0200 Subject: [PATCH 4/6] From Silvia: Unique pointers to the tmva readers, indentation, semicolon, remove this-> --- .../interface/EcalDeadChannelRecoveryBDTG.h | 6 +++--- .../src/EcalDeadChannelRecoveryBDTG.cc | 14 +++++++------- .../EcalRecProducers/python/ecalRecHit_cfi.py | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h index 58faf836b6594..0ba3d8e0233d7 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryBDTG.h @@ -25,7 +25,7 @@ class EcalDeadChannelRecoveryBDTG { ~EcalDeadChannelRecoveryBDTG(); void setParameters(const edm::ParameterSet &ps); - void setCaloTopology(const CaloTopology *topo) { topology_ = topo; }; + void setCaloTopology(const CaloTopology *topo) { topology_ = topo; } double recover( const DetIdT id, const EcalRecHitCollection &hit_collection, double single8Cut, double sum8Cut, bool *acceptFlag); @@ -45,8 +45,8 @@ class EcalDeadChannelRecoveryBDTG { edm::FileInPath bdtWeightFileNoCracks_; edm::FileInPath bdtWeightFileCracks_; - TMVA::Reader *readerNoCrack; - TMVA::Reader *readerCrack; + std::unique_ptr readerNoCrack; + std::unique_ptr readerCrack; }; #endif diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc index feec3461877f7..e056e93283043 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc @@ -30,14 +30,14 @@ void EcalDeadChannelRecoveryBDTG::addVariables(TMVA::Reader *reader) { } template <> void EcalDeadChannelRecoveryBDTG::loadFile() { - readerNoCrack = new TMVA::Reader("!Color:!Silent"); - readerCrack = new TMVA::Reader("!Color:!Silent"); + readerNoCrack = std::unique_ptr (new TMVA::Reader("!Color:!Silent")); + readerCrack = std::unique_ptr (new TMVA::Reader("!Color:!Silent")); - this->addVariables(readerNoCrack); - this->addVariables(readerCrack); + addVariables(readerNoCrack.get()); + addVariables(readerCrack.get()); - reco::details::loadTMVAWeights(readerNoCrack, "BDTG", bdtWeightFileNoCracks_.fullPath()); - reco::details::loadTMVAWeights(readerCrack, "BDTG", bdtWeightFileCracks_.fullPath()); + reco::details::loadTMVAWeights(readerNoCrack.get(), "BDTG", bdtWeightFileNoCracks_.fullPath()); + reco::details::loadTMVAWeights(readerCrack.get(), "BDTG", bdtWeightFileCracks_.fullPath()); } template @@ -51,7 +51,7 @@ void EcalDeadChannelRecoveryBDTG::setParameters(const edm::ParameterSet bdtWeightFileNoCracks_ = ps.getParameter("bdtWeightFileNoCracks"); bdtWeightFileCracks_ = ps.getParameter("bdtWeightFileCracks"); - this->loadFile(); + loadFile(); } template <> diff --git a/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py b/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py index 3f3b8c72641d5..16ce8d61708c5 100644 --- a/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py +++ b/RecoLocalCalo/EcalRecProducers/python/ecalRecHit_cfi.py @@ -49,7 +49,7 @@ # for channel recovery algoRecover = cms.string("EcalRecHitWorkerRecover"), - recoverEBIsolatedChannels = cms.bool(False),##default is false + recoverEBIsolatedChannels = cms.bool(False),##default is false recoverEEIsolatedChannels = cms.bool(False), recoverEBVFE = cms.bool(False), recoverEEVFE = cms.bool(False), From 155b8aa9924ce93eadf4d6240651cd4fb3eb094f Mon Sep 17 00:00:00 2001 From: nancy Date: Wed, 31 Jul 2019 15:22:24 +0200 Subject: [PATCH 5/6] code formatting --- .../src/EcalDeadChannelRecoveryBDTG.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc index e056e93283043..8f2cee47c674c 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryBDTG.cc @@ -30,8 +30,8 @@ void EcalDeadChannelRecoveryBDTG::addVariables(TMVA::Reader *reader) { } template <> void EcalDeadChannelRecoveryBDTG::loadFile() { - readerNoCrack = std::unique_ptr (new TMVA::Reader("!Color:!Silent")); - readerCrack = std::unique_ptr (new TMVA::Reader("!Color:!Silent")); + readerNoCrack = std::unique_ptr(new TMVA::Reader("!Color:!Silent")); + readerCrack = std::unique_ptr(new TMVA::Reader("!Color:!Silent")); addVariables(readerNoCrack.get()); addVariables(readerCrack.get()); From edab6bcf624dc40f832a6749cef7b90df58102ed Mon Sep 17 00:00:00 2001 From: nancy Date: Wed, 31 Jul 2019 16:49:53 +0200 Subject: [PATCH 6/6] Remove algo as argument from setTopology --- .../interface/EcalDeadChannelRecoveryAlgos.h | 2 +- .../src/EcalDeadChannelRecoveryAlgos.cc | 2 +- .../test/plugins/EcalDeadChannelRecoveryProducers.cc | 3 +-- .../EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h index 6f2ec2746f05e..a94f55508e3ca 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/interface/EcalDeadChannelRecoveryAlgos.h @@ -17,7 +17,7 @@ template class EcalDeadChannelRecoveryAlgos { public: void setParameters(const edm::ParameterSet &ps); - void setCaloTopology(std::string algo, const CaloTopology *topology); + void setCaloTopology(const CaloTopology *topology); float correct(const DetIdT id, const EcalRecHitCollection &hit_collection, std::string algo, diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc index 3f1e3f40b21ce..6c09ffe12f528 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/src/EcalDeadChannelRecoveryAlgos.cc @@ -19,7 +19,7 @@ void EcalDeadChannelRecoveryAlgos::setParameters(const edm::ParameterSet &ps) } template -void EcalDeadChannelRecoveryAlgos::setCaloTopology(std::string algo, const CaloTopology *topo) { +void EcalDeadChannelRecoveryAlgos::setCaloTopology(const CaloTopology *topo) { bdtg_.setCaloTopology(topo); } diff --git a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc index 47bcbd312779e..3233f3441b9f5 100644 --- a/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc +++ b/RecoLocalCalo/EcalDeadChannelRecoveryAlgos/test/plugins/EcalDeadChannelRecoveryProducers.cc @@ -67,8 +67,7 @@ void EcalDeadChannelRecoveryProducers::produce(edm::Event& evt, const ed // create a unique_ptr to a EcalRecHitCollection, copy the RecHits into it and // put it in the Event: auto redCollection = std::make_unique(); - std::string dummy; - deadChannelCorrector.setCaloTopology(dummy, theCaloTopology.product()); + deadChannelCorrector.setCaloTopology(theCaloTopology.product()); // // Double loop over EcalRecHit collection and "dead" cell RecHits. diff --git a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc index 35cd1cf323907..cf3dc55d616ad 100644 --- a/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc +++ b/RecoLocalCalo/EcalRecProducers/plugins/EcalRecHitWorkerRecover.cc @@ -124,7 +124,7 @@ bool EcalRecHitWorkerRecover::run(const edm::Event& evt, if (flags == EcalRecHitWorkerRecover::EB_single) { // recover as single dead channel - ebDeadChannelCorrector.setCaloTopology(singleRecoveryMethod_, caloTopology_.product()); + ebDeadChannelCorrector.setCaloTopology(caloTopology_.product()); // channel recovery. Accepted new RecHit has the flag AcceptRecHit=TRUE bool AcceptRecHit = true; @@ -142,7 +142,7 @@ bool EcalRecHitWorkerRecover::run(const edm::Event& evt, } else if (flags == EcalRecHitWorkerRecover::EE_single) { // recover as single dead channel - eeDeadChannelCorrector.setCaloTopology(singleRecoveryMethod_, caloTopology_.product()); + eeDeadChannelCorrector.setCaloTopology(caloTopology_.product()); // channel recovery. Accepted new RecHit has the flag AcceptRecHit=TRUE bool AcceptRecHit = true;