Seed TICL by L1 e/gamma + other timing considerations for e/gamma HLT #32978
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,121 @@ | ||
| /* | ||
| Author: Swagata Mukherjee | ||
| Date: Feb 2021 | ||
| At the time of writing this new module, it is intended to be used mainly for | ||
| phase-2. Before feeding in the L1 e/g collection to | ||
| HLTEcalRecHitInAllL1RegionsProducer, it can pass through this module which | ||
| will filter the collection based on hardware quality and pT. | ||
| The most generic L1 e/g phase-2 collections are: | ||
| TkEm, which is std::vector<l1t::TkEm> | ||
| & | ||
| StaEG, which is BXVector<l1t::EGamma> | ||
| Despite this technical difference, the objects are almost identical, for all | ||
| practical purposes. So any of these two collections could have been used. | ||
| Currently, BXVector<l1t::EGamma> is recognised by the next step | ||
| HLTEcalRecHitInAllL1RegionsProducer, while std::vector<l1t::TkEm> is not. So | ||
| using BXVector<l1t::EGamma> is straightforward. If for some reason one need to | ||
| use std::vector<l1t::TkEm>, changes in HLTEcalRecHitInAllL1RegionsProducer | ||
| would also be necesary. | ||
| */ | ||
|
|
||
| #include "DataFormats/L1TCorrelator/interface/TkEm.h" | ||
| #include "FWCore/Framework/interface/ESHandle.h" | ||
| #include "FWCore/Framework/interface/Event.h" | ||
| #include "FWCore/Framework/interface/EventSetup.h" | ||
| #include "FWCore/Framework/interface/EventSetupRecord.h" | ||
| #include "FWCore/Framework/interface/Frameworkfwd.h" | ||
| #include "FWCore/Framework/interface/MakerMacros.h" | ||
| #include "FWCore/Framework/interface/global/EDProducer.h" | ||
| #include "FWCore/MessageLogger/interface/MessageLogger.h" | ||
| #include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h" | ||
| #include "FWCore/ParameterSet/interface/ParameterSet.h" | ||
| #include "FWCore/ParameterSet/interface/ParameterSetDescription.h" | ||
|
|
||
| class L1TEGammaFilteredCollectionProducer : public edm::global::EDProducer<> { | ||
| public: | ||
| explicit L1TEGammaFilteredCollectionProducer(const edm::ParameterSet&); | ||
| ~L1TEGammaFilteredCollectionProducer() override; | ||
| static void fillDescriptions(edm::ConfigurationDescriptions& descriptions); | ||
| void produce(edm::StreamID sid, edm::Event& iEvent, const edm::EventSetup& iSetup) const override; | ||
|
|
||
| private: | ||
| edm::InputTag l1EgTag_; | ||
| edm::EDGetTokenT<BXVector<l1t::EGamma>> l1EgToken_; | ||
| int quality_; | ||
| bool qualIsMask_; | ||
| bool applyQual_; | ||
| int minBX_; | ||
| int maxBX_; | ||
| double minPt_; | ||
| std::vector<double> scalings_; // pT scaling factors | ||
| double getOfflineEt(double et) const; | ||
| }; | ||
|
|
||
| L1TEGammaFilteredCollectionProducer::L1TEGammaFilteredCollectionProducer(const edm::ParameterSet& iConfig) | ||
| : l1EgTag_(iConfig.getParameter<edm::InputTag>("inputTag")), l1EgToken_(consumes<BXVector<l1t::EGamma>>(l1EgTag_)) { | ||
| quality_ = iConfig.getParameter<int>("quality"); | ||
| qualIsMask_ = iConfig.getParameter<bool>("qualIsMask"); | ||
| applyQual_ = iConfig.getParameter<bool>("applyQual"); | ||
| minBX_ = iConfig.getParameter<int>("minBX"); | ||
| maxBX_ = iConfig.getParameter<int>("maxBX"); | ||
| minPt_ = iConfig.getParameter<double>("minPt"); | ||
| scalings_ = iConfig.getParameter<std::vector<double>>("scalings"); | ||
|
|
||
| produces<BXVector<l1t::EGamma>>(); | ||
| } | ||
|
|
||
| L1TEGammaFilteredCollectionProducer::~L1TEGammaFilteredCollectionProducer() = default; | ||
|
|
||
| void L1TEGammaFilteredCollectionProducer::fillDescriptions(edm::ConfigurationDescriptions& descriptions) { | ||
| edm::ParameterSetDescription desc; | ||
| desc.add<edm::InputTag>("inputTag", edm::InputTag("L1EGammaClusterEmuProducer")); | ||
| desc.add<int>("quality", 0x2); | ||
| desc.add<bool>("qualIsMask", true); | ||
| desc.add<bool>("applyQual", true); | ||
| desc.add<int>("minBX", -1); | ||
| desc.add<int>("maxBX", 1); | ||
| desc.add<double>("minPt", 5.0); | ||
| desc.add<std::vector<double>>("scalings", {2.6604, 1.06077, 0.0}); | ||
| descriptions.add("L1TEGammaFilteredCollectionProducer", desc); | ||
| } | ||
|
|
||
| void L1TEGammaFilteredCollectionProducer::produce(edm::StreamID sid, | ||
| edm::Event& iEvent, | ||
| const edm::EventSetup& iSetup) const { | ||
| auto outEgs = std::make_unique<BXVector<l1t::EGamma>>(); | ||
| auto l1Egs = iEvent.getHandle(l1EgToken_); | ||
|
|
||
| int startBX = std::max((*l1Egs).getFirstBX(), minBX_); | ||
| int endBX = std::min((*l1Egs).getLastBX(), maxBX_); | ||
|
|
||
| for (int bx = startBX; bx <= endBX; bx++) { | ||
| // Loop over all L1 e/gamma objects | ||
| for (BXVector<l1t::EGamma>::const_iterator iEg = (*l1Egs).begin(bx); iEg != (*l1Egs).end(bx); iEg++) { | ||
| double offlineEt = this->getOfflineEt((*iEg).pt()); | ||
| bool passQuality(false); | ||
| if (applyQual_) { | ||
| if (qualIsMask_) | ||
| passQuality = ((*iEg).hwQual() & quality_); | ||
| else | ||
| passQuality = ((*iEg).hwQual() == quality_); | ||
| } else | ||
| passQuality = true; | ||
|
|
||
| // if quality is passed, put the object in filtered collection | ||
| if (passQuality && (offlineEt > minPt_)) { | ||
| outEgs->push_back(bx, *iEg); | ||
| } | ||
| } // l1EG loop ends | ||
| } // BX loop ends | ||
| iEvent.put(std::move(outEgs)); | ||
| } | ||
|
|
||
| double L1TEGammaFilteredCollectionProducer::getOfflineEt(double et) const { | ||
| return (scalings_.at(0) + et * scalings_.at(1) + et * et * scalings_.at(2)); | ||
| } | ||
|
|
||
| DEFINE_FWK_MODULE(L1TEGammaFilteredCollectionProducer); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /* | ||
| Author: Swagata Mukherjee | ||
|
|
||
| Date: Feb 2021 | ||
|
|
||
| TICL is currently seeded by tracks, or just globally. | ||
| Here, adding option to seed TICL by L1 e/gamma objects (L1 TkEm). | ||
| This is expected to be useful for CPU timing at the HLT. | ||
| */ | ||
|
|
||
| #include "SeedingRegionByL1.h" | ||
|
|
||
| #include <algorithm> | ||
| #include <set> | ||
| #include <vector> | ||
|
|
||
| #include "FWCore/MessageLogger/interface/MessageLogger.h" | ||
|
|
||
| ticl::SeedingRegionByL1::SeedingRegionByL1(const edm::ParameterSet &conf, edm::ConsumesCollector &sumes) | ||
| : SeedingRegionAlgoBase(conf, sumes), | ||
| l1TkEmsToken_(sumes.consumes<std::vector<l1t::TkEm>>(conf.getParameter<edm::InputTag>("l1TkEmColl"))), | ||
| algoVerbosity_(conf.getParameter<int>("algo_verbosity")), | ||
| minPt_(conf.getParameter<double>("minPt")), | ||
| minAbsEta_(conf.getParameter<double>("minAbsEta")), | ||
| maxAbsEta_(conf.getParameter<double>("maxAbsEta")), | ||
| endcapScalings_(conf.getParameter<std::vector<double>>("endcapScalings")), | ||
| quality_(conf.getParameter<int>("quality")) {} | ||
|
|
||
| void ticl::SeedingRegionByL1::makeRegions(const edm::Event &ev, | ||
| const edm::EventSetup &es, | ||
| std::vector<TICLSeedingRegion> &result) { | ||
| auto l1TrkEms = ev.getHandle(l1TkEmsToken_); | ||
| edm::ProductID l1tkemsId = l1TrkEms.id(); | ||
|
|
||
| for (size_t indx = 0; indx < (*l1TrkEms).size(); indx++) { | ||
| const auto &l1TrkEm = (*l1TrkEms)[indx]; | ||
| double offlinePt = this->TkEmOfflineEt(l1TrkEm.pt()); | ||
| if ((offlinePt < minPt_) || (std::abs(l1TrkEm.eta()) < minAbsEta_) || (std::abs(l1TrkEm.eta()) > maxAbsEta_) || | ||
| (l1TrkEm.EGRef()->hwQual() != quality_)) { | ||
| continue; | ||
| } | ||
|
|
||
| int iSide = int(l1TrkEm.eta() > 0); | ||
| result.emplace_back(GlobalPoint(l1TrkEm.p4().X(), l1TrkEm.p4().Y(), l1TrkEm.p4().Z()), | ||
| GlobalVector(l1TrkEm.px(), l1TrkEm.py(), l1TrkEm.pz()), | ||
| iSide, | ||
| indx, | ||
| l1tkemsId); | ||
| } | ||
|
|
||
| std::sort(result.begin(), result.end(), [](const TICLSeedingRegion &a, const TICLSeedingRegion &b) { | ||
| return a.directionAtOrigin.perp2() > b.directionAtOrigin.perp2(); | ||
| }); | ||
| } | ||
|
|
||
| double ticl::SeedingRegionByL1::TkEmOfflineEt(double Et) const { | ||
| return (endcapScalings_.at(0) + Et * endcapScalings_.at(1) + Et * Et * endcapScalings_.at(2)); | ||
| } | ||
|
|
||
| void ticl::SeedingRegionByL1::fillPSetDescription(edm::ParameterSetDescription &desc) { | ||
| desc.add<edm::InputTag>("l1TkEmColl", edm::InputTag("L1TkPhotonsHGC", "EG")); | ||
| desc.add<int>("algo_verbosity", 0); | ||
| desc.add<double>("minPt", 10); | ||
| desc.add<double>("minAbsEta", 1.479); | ||
| desc.add<double>("maxAbsEta", 4.0); | ||
| desc.add<std::vector<double>>("endcapScalings", {3.17445, 1.13219, 0.0}); | ||
| desc.add<int>("quality", 5); | ||
| } | ||
|
|
||
| edm::ParameterSetDescription ticl::SeedingRegionByL1::makePSetDescription() { | ||
| edm::ParameterSetDescription desc; | ||
| fillPSetDescription(desc); | ||
| return desc; | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,43 @@ | ||||||
| // Author: Swagata Mukherjee | ||||||
|
|
||||||
| #ifndef RecoHGCal_TICL_SeedingRegionByL1_h | ||||||
| #define RecoHGCal_TICL_SeedingRegionByL1_h | ||||||
| #include <memory> | ||||||
| #include <string> | ||||||
|
|
||||||
| #include "DataFormats/L1TCorrelator/interface/TkEm.h" | ||||||
| #include "DataFormats/L1TCorrelator/interface/TkEmFwd.h" | ||||||
| #include "FWCore/Framework/interface/ConsumesCollector.h" | ||||||
| #include "FWCore/Framework/interface/ESHandle.h" | ||||||
| #include "FWCore/Framework/interface/Event.h" | ||||||
| #include "FWCore/Framework/interface/Frameworkfwd.h" | ||||||
| #include "FWCore/Framework/interface/MakerMacros.h" | ||||||
| #include "FWCore/MessageLogger/interface/MessageLogger.h" | ||||||
| #include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h" | ||||||
| #include "FWCore/ParameterSet/interface/ParameterSetDescription.h" | ||||||
| #include "FWCore/Utilities/interface/ESGetToken.h" | ||||||
| #include "RecoHGCal/TICL/plugins/SeedingRegionAlgoBase.h" | ||||||
|
|
||||||
| namespace ticl { | ||||||
| class SeedingRegionByL1 final : public SeedingRegionAlgoBase { | ||||||
| public: | ||||||
| SeedingRegionByL1(const edm::ParameterSet& conf, edm::ConsumesCollector& sumes); | ||||||
|
|
||||||
| void initialize(const edm::EventSetup& es) override{}; | ||||||
| void makeRegions(const edm::Event& ev, const edm::EventSetup& es, std::vector<TICLSeedingRegion>& result) override; | ||||||
| static void fillPSetDescription(edm::ParameterSetDescription& desc); | ||||||
| static edm::ParameterSetDescription makePSetDescription(); | ||||||
|
There was a problem hiding this comment. Please, see my comments here. There was a problem hiding this comment. Hello @rovere, cmssw/RecoHGCal/TICL/plugins/TICLSeedingRegionProducer.cc Lines 60 to 78 in 20cffd1 i.e, the parameters needed by SeedingRegionByL1 class are included explicitly in the TICLSeedingRegionProducer::fillDescriptions ? There was a problem hiding this comment. Ciao @swagata87 I meant following the example of what has been done, e.g, here:
All that is described in more details here There was a problem hiding this comment. So, ultimately this is the way to go. But this puts us in a tricky situation, we would rather very much like to have this in a release built tomorrow. So the way I see it there are two ways
There was a problem hiding this comment. Ciao @Sam-Harper There was a problem hiding this comment. so we updated the config, there are few things to address: the name of plugin pset, we chose "seedingPSet" the name of the parameter which is the plugin name, "type" was the example but its not great to use a python reserved word (although in this case we dont shadow). They can be changed to anything people want. There was a problem hiding this comment. @rovere any comments on the parameter names ? There was a problem hiding this comment. The name is fine. There was a problem hiding this comment. no problem it ended up being easier than we thought it would be at first glance :) There was a problem hiding this comment.
Thank you @rovere for your suggestion to use plugins, and also for the examples & twiki that you linked to before. |
||||||
|
|
||||||
| private: | ||||||
| edm::EDGetTokenT<std::vector<l1t::TkEm>> l1TkEmsToken_; | ||||||
| int algoVerbosity_ = 0; | ||||||
| double minPt_; // minimum pT of L1 TkEm objects | ||||||
| double minAbsEta_; // minimum |eta| of L1 TkEm objects | ||||||
| double maxAbsEta_; // maximum |eta| of L1 TkEm objects | ||||||
| std::vector<double> endcapScalings_; // pT scaling factors for endcap | ||||||
| int quality_; // hwQual | ||||||
|
|
||||||
| double TkEmOfflineEt(double Et) const; | ||||||
|
There was a problem hiding this comment. Sorry to be picky
Suggested change
to agree with 2.8 https://cms-sw.github.io/cms_coding_rules.html (and function parameters commonly start with lowercase too) |
||||||
| }; | ||||||
| } // namespace ticl | ||||||
| #endif | ||||||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method and its declaration should not be necessary and can be removed, IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm yes, it was useful in the original design but in the current design I agree its not necessary and introduces a difference to the other SeedingRegions which violates principle of least surprise. @swagata87 , can you remove it please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makePSetDescription is now removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you Swagata