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.

Sign up for GitHub

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

Jump to bottom

bsunanda:Run2 alca14 Get rid of unused variables and avoid explicit names #9062

Merged
merged 5 commits into from May 19, 2015
Merged

bsunanda:Run2 alca14 Get rid of unused variables and avoid explicit names #9062

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eac62e1
Take care of Salvatore comments of not using ad-hoc names
May 12, 2015
bbfd0a9
Put the name in the cfi file
May 13, 2015
56239d0
A few test python files which were not corrected
May 14, 2015
aecef55
one missed out
May 14, 2015
b7c52a2
Avoid counting twice
May 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
68 changes: 40 additions & 28 deletions Calibration/HcalAlCaRecoProducers/src/AlCaGammaJetProducer.cc
View file
Open in desktop
Expand Up @@ -47,7 +47,7 @@ class AlCaGammaJetProducer : public edm::EDProducer {
virtual void endJob();

private:
bool select (reco::PhotonCollection, reco::PFJetCollection);
bool select(const reco::PhotonCollection&, const reco::PFJetCollection&);

// ----------member data ---------------------------

Expand Down Expand Up @@ -104,9 +104,9 @@ AlCaGammaJetProducer::AlCaGammaJetProducer(const edm::ParameterSet& iConfig) : n
tok_loosePhoton_ = consumes<edm::ValueMap<Bool_t> >(labelLoosePhot_);
tok_tightPhoton_ = consumes<edm::ValueMap<Bool_t> >(labelTightPhot_);
tok_GsfElec_ = consumes<reco::GsfElectronCollection>(labelGsfEle_);
tok_Rho_ = consumes<double>(labelRho_);
tok_Conv_ = consumes<reco::ConversionCollection>(labelConv_);
tok_BS_ = consumes<reco::BeamSpot>(labelBeamSpot_);
tok_Rho_ = consumes<double>(labelRho_);
tok_Conv_ = consumes<reco::ConversionCollection>(labelConv_);
tok_BS_ = consumes<reco::BeamSpot>(labelBeamSpot_);

// register your products
produces<reco::PhotonCollection>(labelPhoton_.encode());
Expand Down Expand Up @@ -135,13 +135,24 @@ void AlCaGammaJetProducer::endJob() {
edm::LogInfo("AlcaGammaJet") << "Accepts " << nSelect_ << " events from a total of " << nAll_ << " events";
}

bool AlCaGammaJetProducer::select (reco::PhotonCollection ph, reco::PFJetCollection jt) {

if (jt.size()<1) return false;
if (ph.size()<1) return false;
if (((jt.at(0)).pt())<minPtJet_) return false;
if (((ph.at(0)).pt())<minPtPhoton_) return false;
return true;
bool AlCaGammaJetProducer::select (const reco::PhotonCollection &ph, const reco::PFJetCollection &jt) {

// Check the requirement for minimum pT
if (ph.size() == 0) return false;
bool ok(false);
for (reco::PFJetCollection::const_iterator itr=jt.begin();
itr!=jt.end(); ++itr) {
if (itr->pt() >= minPtJet_) {
ok = true;
break;
}
}
if (!ok) return ok;
for (reco::PhotonCollection::const_iterator itr=ph.begin();
itr!=ph.end(); ++itr) {
if (itr->pt() >= minPtPhoton_) return ok;
}
return false;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

@bsunanda IIUC the tests you perform are:

  1. Test if the Photon collection is empty: return false if so;
  2. Check the requirement for minimum pT: at least one PFJet should have pt larger than the threshold, otherwise return false;
  3. Check the requirement for minimum pT: at least one Photon should have pt larger than the threshold. If so, return true, otherwise false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes - we need events with at least one high pt jet and one high pt photon. The assumption of ordering was found to be incorrect - so the code is optimized this way.

From: Salvatore Di Guida [notifications@github.com]
Sent: 13 May 2015 10:17
To: cms-sw/cmssw
Cc: Sunanda Banerjee
Subject: Re: [cmssw] bsunanda:Run2 alca13 Get rid of unused variables and avoid explicit names (#9062)

In Calibration/HcalAlCaRecoProducers/src/AlCaGammaJetProducer.cchttps://github.com//pull/9062#discussion_r30210248:

  • // Check the requirement for minimum pT
  • if (ph.size() == 0) return false;
  • bool ok(false);
  • for (reco::PFJetCollection::const_iterator itr=jt.begin();
  •   itr!=jt.end(); ++itr) {
  • if (itr->pt() >= minPtJet_) {
  •  ok = true;

  •  break;
  • }
  • }
  • if (!ok) return ok;
  • for (reco::PhotonCollection::const_iterator itr=ph.begin();
  •   itr!=ph.end(); ++itr) {
  • if (itr->pt() >= minPtPhoton_) return ok;
  • }
  • return false;
    }

@bsunandahttps://github.com/bsunanda IIUC the tests you perform are:

  1. Test if the Photon collection is empty: return false if so;
  2. Check the requirement for minimum pT: at least one PFJet should have pt larger than the threshold, otherwise return false;
  3. Check the requirement for minimum pT: at least one Photon should have pt larger than the threshold. If so, return true, otherwise false.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/9062/files#r30210248.

// ------------ method called to produce the data ------------
void AlCaGammaJetProducer::produce(edm::Event& iEvent, const edm::EventSetup& iSetup) {
Expand Down Expand Up @@ -275,6 +286,7 @@ void AlCaGammaJetProducer::produce(edm::Event& iEvent, const edm::EventSetup& iS
std::auto_ptr<std::vector<Bool_t> > miniLoosePhoton(new std::vector<Bool_t>());
std::auto_ptr<std::vector<Bool_t> > miniTightPhoton(new std::vector<Bool_t>());


// See if this event is useful
bool accept = select(photon, pfjets);
if (accept) {
Expand Down Expand Up @@ -354,25 +366,25 @@ void AlCaGammaJetProducer::produce(edm::Event& iEvent, const edm::EventSetup& iS
}
}
}

//Put them in the event
iEvent.put( miniPhotonCollection, labelPhoton_.encode());
iEvent.put( miniPFjetCollection, labelPFJet_.encode());
iEvent.put( miniHBHECollection, labelHBHE_.encode());
iEvent.put( miniHFCollection, labelHF_.encode());
iEvent.put( miniHOCollection, labelHO_.encode());
iEvent.put( miniTriggerCollection, labelTrigger_.encode());
iEvent.put( miniPFCandCollection, labelPFCandidate_.encode());
iEvent.put( miniVtxCollection, labelVertex_.encode());
iEvent.put( miniPFMETCollection, labelPFMET_.encode());
iEvent.put( miniGSFeleCollection, labelGsfEle_.encode());
iEvent.put( miniRhoCollection, labelRho_.encode());
iEvent.put( miniConversionCollection, labelConv_.encode());
iEvent.put( miniBeamSpotCollection, labelBeamSpot_.encode());
iEvent.put( miniLoosePhoton, labelLoosePhot_.encode());
iEvent.put( miniTightPhoton, labelTightPhot_.encode());
}

//Put them in the event
iEvent.put( miniPhotonCollection, labelPhoton_.encode());
iEvent.put( miniPFjetCollection, labelPFJet_.encode());
iEvent.put( miniHBHECollection, labelHBHE_.encode());
iEvent.put( miniHFCollection, labelHF_.encode());
iEvent.put( miniHOCollection, labelHO_.encode());
iEvent.put( miniTriggerCollection, labelTrigger_.encode());
iEvent.put( miniPFCandCollection, labelPFCandidate_.encode());
iEvent.put( miniVtxCollection, labelVertex_.encode());
iEvent.put( miniPFMETCollection, labelPFMET_.encode());
iEvent.put( miniGSFeleCollection, labelGsfEle_.encode());
iEvent.put( miniRhoCollection, labelRho_.encode());
iEvent.put( miniConversionCollection, labelConv_.encode());
iEvent.put( miniBeamSpotCollection, labelBeamSpot_.encode());
iEvent.put( miniLoosePhoton, labelLoosePhot_.encode());
iEvent.put( miniTightPhoton, labelTightPhot_.encode());

return;

}
Expand Down
4 changes: 2 additions & 2 deletions Calibration/HcalAlCaRecoProducers/src/AlCaHBHEMuonProducer.cc
View file
Open in desktop
Expand Up @@ -95,8 +95,8 @@ AlCaHBHEMuonProducer::AlCaHBHEMuonProducer(const edm::ParameterSet& iConfig) :
//saves the following collections
produces<reco::BeamSpot>(labelBS_.label());
produces<reco::VertexCollection>(labelVtx_.label());
produces<EcalRecHitCollection>("EcalRecHitsEB");
produces<EcalRecHitCollection>("EcalRecHitsEE");
produces<EcalRecHitCollection>(labelEB_.instance());
produces<EcalRecHitCollection>(labelEE_.instance());
produces<HBHERecHitCollection>(labelHBHE_.label());
produces<reco::MuonCollection>(labelMuon_.label());
}
Expand Down
12 changes: 7 additions & 5 deletions Calibration/HcalAlCaRecoProducers/src/AlCaIsoTracksProducer.cc
View file
Open in desktop
Expand Up @@ -104,7 +104,7 @@ class AlCaIsoTracksProducer : public edm::EDProducer {
int nRun, nAll, nGood;
edm::InputTag labelTriggerEvent_, labelTriggerResults_;
edm::InputTag labelGenTrack_, labelRecVtx_, labelHltGT_;
edm::InputTag labelEB_, labelEE_, labelHBHE_;
edm::InputTag labelEB_, labelEE_, labelHBHE_, labelBS_;
const MagneticField *bField;
const CaloGeometry *geo;
double ptL1, etaL1, phiL1;
Expand Down Expand Up @@ -149,6 +149,7 @@ AlCaIsoTracksProducer::AlCaIsoTracksProducer(const edm::ParameterSet& iConfig) :
eIsolation_ = iConfig.getParameter<double>("IsolationEnergy");
labelGenTrack_ = iConfig.getParameter<edm::InputTag>("TrackLabel");
labelRecVtx_ = iConfig.getParameter<edm::InputTag>("VertexLabel");
labelBS_ = iConfig.getParameter<edm::InputTag>("BeamSpotLabel");
labelEB_ = iConfig.getParameter<edm::InputTag>("EBRecHitLabel");
labelEE_ = iConfig.getParameter<edm::InputTag>("EERecHitLabel");
labelHBHE_ = iConfig.getParameter<edm::InputTag>("HBHERecHitLabel");
Expand All @@ -162,7 +163,7 @@ AlCaIsoTracksProducer::AlCaIsoTracksProducer(const edm::ParameterSet& iConfig) :
tok_trigRes_ = consumes<edm::TriggerResults>(labelTriggerResults_);
tok_genTrack_ = consumes<reco::TrackCollection>(labelGenTrack_);
tok_recVtx_ = consumes<reco::VertexCollection>(labelRecVtx_);
tok_bs_ = consumes<reco::BeamSpot>(edm::InputTag("offlineBeamSpot"));
tok_bs_ = consumes<reco::BeamSpot>(labelBS_);
tok_EB_ = consumes<EcalRecHitCollection>(labelEB_);
tok_EE_ = consumes<EcalRecHitCollection>(labelEE_);
tok_hbhe_ = consumes<HBHERecHitCollection>(labelHBHE_);
Expand Down Expand Up @@ -195,10 +196,11 @@ AlCaIsoTracksProducer::AlCaIsoTracksProducer(const edm::ParameterSet& iConfig) :
trigKount = trigPass = dummy;

//create also IsolatedPixelTrackCandidateCollection which contains isolation info and reference to primary track
produces<reco::HcalIsolatedTrackCandidateCollection>("HcalIsolatedTrackCollection");
static const std::string labelIsoTk = "HcalIsolatedTrackCollection";
Copy link
Contributor

Choose a reason for hiding this comment

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

@bsunanda almost there.
There is still the magic string HcalIsolatedTrackCollection in the Event::put call in the produce method. See
https://github.com/bsunanda/cmssw/blob/Run2-alca14/Calibration/HcalAlCaRecoProducers/src/AlCaIsoTracksProducer.cc#L340
The std::string labelIsoTk should be not a const in the c'tor body, but a static const data member of the producer.
Something like:

 // producer interface
 //...
  edm::EDGetTokenT<EcalRecHitCollection>                  tok_EE_;
  edm::EDGetTokenT<HBHERecHitCollection>                  tok_hbhe_;
  static const std::string labelIsoTk;

// producer body
const std::string AlCaIsoTracksProducer::labelIsoTk = "HcalIsolatedTrackCollection";

AlCaIsoTracksProducer::AlCaIsoTracksProducer(const edm::ParameterSet& iConfig)
// in the c'tor body, remove the declaration in this line
//...
  edm::LogInfo("HcalIsoTrack") << " Expected to produce the collections:\n"
                   << "reco::HcalIsolatedTrackCandidateCollection "
                   << " with label " << labelIsoTk << "\n"
                   << "reco::VertexCollection with label " << labelRecVtx_.label() << "\n"
                   << "EcalRecHitCollection with label EcalRecHitsEB\n"
                   << "EcalRecHitCollection with label EcalRecHitsEE\n"
                   << "HBHERecHitCollection with label " << labelHBHE_.label();

void AlCaIsoTracksProducer::produce(edm::Event& iEvent, const edm::EventSetup& iSetup)
// in the produce method, use the string for designating the collection to be put in the event
  iEvent.put(outputHcalIsoTrackColl, labelIsoTk);

produces<reco::HcalIsolatedTrackCandidateCollection>(labelIsoTk);
produces<reco::VertexCollection>(labelRecVtx_.label());
produces<EcalRecHitCollection>("EcalRecHitsEB");
produces<EcalRecHitCollection>("EcalRecHitsEE");
produces<EcalRecHitCollection>(labelEB_.instance());
produces<EcalRecHitCollection>(labelEE_.instance());
produces<HBHERecHitCollection>(labelHBHE_.label());

edm::LogInfo("HcalIsoTrack") << " Expected to produce the collections:\n"
Expand Down
64 changes: 43 additions & 21 deletions Calibration/IsolatedParticles/plugins/HcalRaddamMuon.cc
View file
Open in desktop
Expand Up @@ -194,18 +194,51 @@ class HcalRaddamMuon : public edm::EDAnalyzer {
std::string theTrackQuality;
edm::InputTag muonsrc_;
std::vector <double> track_cosmic_xposition , track_cosmic_yposition, track_cosmic_zposition, track_cosmic_xmomentum,track_cosmic_ymomentum, track_cosmic_zmomentum, track_cosmic_rad, track_cosmic_detid;
};

edm::EDGetTokenT<edm::PCaloHitContainer> tok_hcal_;
edm::EDGetTokenT<edm::TriggerResults> tok_trigRes_;
edm::EDGetTokenT<reco::VertexCollection> tok_recVtx_;
edm::EDGetTokenT<reco::BeamSpot> tok_bs_;
edm::EDGetTokenT<EcalRecHitCollection> tok_EB_;
edm::EDGetTokenT<EcalRecHitCollection> tok_EE_;
edm::EDGetTokenT<HBHERecHitCollection> tok_hbhe_;
edm::EDGetTokenT<reco::MuonCollection> tok_muon_;
};

HcalRaddamMuon::HcalRaddamMuon(const edm::ParameterSet& iConfig) {
//now do what ever initialization is needed
HLTriggerResults_ = iConfig.getUntrackedParameter<edm::InputTag>("HLTriggerResults_");
muonsrc_ = iConfig.getUntrackedParameter<edm::InputTag>("MuonSource");
verbosity_ = iConfig.getUntrackedParameter<int>("Verbosity",0);
isAOD_ = iConfig.getUntrackedParameter<bool>("IsAOD",false);
isSLHC_ = iConfig.getUntrackedParameter<bool>("IsSLHC",true);
maxDepth_ = iConfig.getUntrackedParameter<int>("MaxDepth",4);
// muonsrc_ = iConfig.getUntrackedParameter<edm::InputTag>("muonsrc");

if (maxDepth_ > 7) maxDepth_ = 7;
else if (maxDepth_ < 1) maxDepth_ = 4;

tok_hcal_ = consumes<edm::PCaloHitContainer>(edm::InputTag("g4SimHits","HcalHits"));
tok_trigRes_ = consumes<edm::TriggerResults>(HLTriggerResults_);
tok_recVtx_ = consumes<reco::VertexCollection>(edm::InputTag("offlinePrimaryVertices"));
tok_bs_ = consumes<reco::BeamSpot>(edm::InputTag("offlineBeamSpot"));
if (isAOD_) {
tok_EB_ = consumes<EcalRecHitCollection>(edm::InputTag("reducedEcalRecHitsEB"));
tok_EE_ = consumes<EcalRecHitCollection>(edm::InputTag("reducedEcalRecHitsEE"));
if (isSLHC_) {
tok_hbhe_= consumes<HBHERecHitCollection>(edm::InputTag("reducedHcalRecHits","hbheUpgradeReco"));
} else {
tok_hbhe_= consumes<HBHERecHitCollection>(edm::InputTag("reducedHcalRecHits", "hbhereco"));
}
} else {
tok_EB_ = consumes<EcalRecHitCollection>(edm::InputTag("ecalRecHit","EcalRecHitsEB"));
tok_EE_ = consumes<EcalRecHitCollection>(edm::InputTag("ecalRecHit","EcalRecHitsEE"));
if (isSLHC_) {
tok_hbhe_= consumes<HBHERecHitCollection>(edm::InputTag("hbheUpgradeReco"));
} else {
tok_hbhe_= consumes<HBHERecHitCollection>(edm::InputTag("hbhereco"));
}
}
tok_muon_ = consumes<reco::MuonCollection>(muonsrc_);
}

HcalRaddamMuon::~HcalRaddamMuon() {
Expand All @@ -229,10 +262,10 @@ void HcalRaddamMuon::analyze(const edm::Event& iEvent, const edm::EventSetup& iS
BXNumber = iEvent.bunchCrossing();

edm::Handle<edm::PCaloHitContainer> calosimhits;
iEvent.getByLabel("g4SimHits","HcalHits",calosimhits);
iEvent.getByToken(tok_hcal_,calosimhits);

edm::Handle<edm::TriggerResults> _Triggers;
iEvent.getByLabel(HLTriggerResults_,_Triggers);
iEvent.getByToken(tok_trigRes_,_Triggers);

if ((verbosity_%10)>1) std::cout << "size of all triggers "
<< all_triggers.size() << std::endl;
Expand Down Expand Up @@ -288,32 +321,21 @@ void HcalRaddamMuon::analyze(const edm::Event& iEvent, const edm::EventSetup& iS
const HcalTopology* theHBHETopology = htopo.product();

edm::Handle<reco::BeamSpot> bmspot;
iEvent.getByLabel("offlineBeamSpot",bmspot);
iEvent.getByToken(tok_bs_,bmspot);

edm::Handle<reco::VertexCollection> vtx;
iEvent.getByLabel("offlinePrimaryVertices",vtx);
iEvent.getByToken(tok_recVtx_,vtx);

edm::Handle<EcalRecHitCollection> barrelRecHitsHandle;
edm::Handle<EcalRecHitCollection> endcapRecHitsHandle;
if (isAOD_) {
iEvent.getByLabel("EcalRecHitsEB",barrelRecHitsHandle);
iEvent.getByLabel("EcalRecHitsEE",endcapRecHitsHandle);
} else {
iEvent.getByLabel("ecalRecHit","EcalRecHitsEB",barrelRecHitsHandle);
iEvent.getByLabel("ecalRecHit","EcalRecHitsEE",endcapRecHitsHandle);
}
iEvent.getByToken(tok_EB_,barrelRecHitsHandle);
iEvent.getByToken(tok_EE_,endcapRecHitsHandle);

edm::Handle<HBHERecHitCollection> hbhe;
if (isSLHC_) {
if (isAOD_) iEvent.getByLabel("reducedHcalRecHits","hbheUpgradeReco",hbhe);
else iEvent.getByLabel("hbheUpgradeReco", hbhe);
} else {
if (isAOD_) iEvent.getByLabel("reducedHcalRecHits","hbhereco",hbhe);
else iEvent.getByLabel("hbhereco", hbhe);
}
iEvent.getByToken(tok_hbhe_,hbhe);

edm::Handle<reco::MuonCollection> _Muon;
iEvent.getByLabel("muons",_Muon);
iEvent.getByToken(tok_muon_,_Muon);
const reco::Vertex& vertex = (*(vtx)->begin());

math::XYZPoint bspot;
Expand Down