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.

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

Adding an HLT filter based on PPS local track multiplicity conditions for selection of events for PPS PCL #34827

Merged
merged 4 commits into from Aug 25, 2021
Merged
Changes from 2 commits
Commits
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
183 changes: 183 additions & 0 deletions HLTrigger/special/plugins/HLTPPSCalFilter.cc
@@ -0,0 +1,183 @@
// <author>Mariana Araujo</author>
// <email>mariana.araujo@cern.ch</email>
// <created>2020-12-30</created>
// <description>
// HLT filter module to select events with min and max multiplicity
// in each tracker of the PPS for PCL
// Adapted from a preexisting filter code by Laurent Forthomme
// </description>

#include "FWCore/Framework/interface/global/EDFilter.h"
#include "FWCore/Framework/interface/Event.h"
#include "FWCore/Framework/interface/EventSetup.h"
#include "FWCore/ParameterSet/interface/ParameterSet.h"
#include "FWCore/ParameterSet/interface/ConfigurationDescriptions.h"
#include "FWCore/MessageLogger/interface/MessageLogger.h"
#include "FWCore/Utilities/interface/InputTag.h"

#include "DataFormats/Common/interface/Ref.h"
#include "DataFormats/Common/interface/Handle.h"
#include "DataFormats/Common/interface/DetSet.h"
#include "DataFormats/Common/interface/DetSetVector.h"

#include "DataFormats/HLTReco/interface/TriggerTypeDefs.h"
#include "DataFormats/HLTReco/interface/TriggerFilterObjectWithRefs.h"

#include "DataFormats/CTPPSDetId/interface/CTPPSPixelDetId.h"

#include "DataFormats/CTPPSReco/interface/CTPPSPixelLocalTrack.h" // pixel
#include "DataFormats/CTPPSReco/interface/TotemRPLocalTrack.h" // strip
#include "DataFormats/CTPPSReco/interface/CTPPSDiamondLocalTrack.h" // diamond

#include <unordered_map>

class HLTPPSCalFilter : public edm::global::EDFilter<> {
public:
explicit HLTPPSCalFilter(const edm::ParameterSet&);
~HLTPPSCalFilter() override = default;

static void fillDescriptions(edm::ConfigurationDescriptions&);
bool filter(edm::StreamID, edm::Event&, const edm::EventSetup&) const override;

private:
edm::InputTag pixelLocalTrackInputTag_; // Input tag identifying the pixel detector
Copy link
Contributor

Choose a reason for hiding this comment

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

All these private class members can be declared const

edm::EDGetTokenT<edm::DetSetVector<CTPPSPixelLocalTrack>> pixelLocalTrackToken_;

edm::InputTag stripLocalTrackInputTag_; // Input tag identifying the strip detector
edm::EDGetTokenT<edm::DetSetVector<TotemRPLocalTrack>> stripLocalTrackToken_;

edm::InputTag diamondLocalTrackInputTag_; // Input tag identifying the diamond detector
edm::EDGetTokenT<edm::DetSetVector<CTPPSDiamondLocalTrack>> diamondLocalTrackToken_;

int minTracks_;
int maxTracks_;

bool do_express_;
};

void HLTPPSCalFilter::fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
edm::ParameterSetDescription desc;

desc.add<edm::InputTag>("pixelLocalTrackInputTag", edm::InputTag("ctppsPixelLocalTracks"))
->setComment("input tag of the pixel local track collection");
desc.add<edm::InputTag>("stripLocalTrackInputTag", edm::InputTag("totemRPLocalTrackFitter"))
->setComment("input tag of the strip local track collection");
desc.add<edm::InputTag>("diamondLocalTrackInputTag", edm::InputTag("ctppsDiamondLocalTracks"))
->setComment("input tag of the diamond local track collection");

desc.add<int>("minTracks", 1)->setComment("minimum number of tracks in pot");
desc.add<int>("maxTracks", -1)->setComment("maximum number of tracks in pot");

desc.add<bool>("do_express", true)->setComment("toggle on filter type; true for Express, false for Prompt");

desc.add<int>("triggerType", trigger::TriggerTrack);

descriptions.add("hltPPSCalFilter", desc);
}

HLTPPSCalFilter::HLTPPSCalFilter(const edm::ParameterSet& iConfig)
: pixelLocalTrackInputTag_(iConfig.getParameter<edm::InputTag>("pixelLocalTrackInputTag")),
pixelLocalTrackToken_(consumes<edm::DetSetVector<CTPPSPixelLocalTrack>>(pixelLocalTrackInputTag_)),

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this empty line can improve readability

stripLocalTrackInputTag_(iConfig.getParameter<edm::InputTag>("stripLocalTrackInputTag")),
stripLocalTrackToken_(consumes<edm::DetSetVector<TotemRPLocalTrack>>(stripLocalTrackInputTag_)),

diamondLocalTrackInputTag_(iConfig.getParameter<edm::InputTag>("diamondLocalTrackInputTag")),
diamondLocalTrackToken_(consumes<edm::DetSetVector<CTPPSDiamondLocalTrack>>(diamondLocalTrackInputTag_)),

minTracks_(iConfig.getParameter<int>("minTracks")),
maxTracks_(iConfig.getParameter<int>("maxTracks")),

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this empty line can improve readability

do_express_(iConfig.getParameter<bool>("do_express")) {}

bool HLTPPSCalFilter::filter(edm::StreamID, edm::Event& iEvent, const edm::EventSetup& iSetup) const {
// Helper tool to count valid tracks
const auto valid_trks = [](const auto& trk) { return trk.isValid(); };

// maps for assigning filter pass / fail
std::unordered_map<uint32_t, bool> pixel_filter_;
std::unordered_map<uint32_t, bool> strip_filter_;
std::unordered_map<uint32_t, bool> diam_filter_;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems these 3 maps are just used as bools to switch on/off IDs, so these variables could become (const) class members and initialised as such in the class, rather than being local variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The maps are used as bools to store whether the track multiplicity of each detector is within the max and min number of tracks. The final filter condition combines the result for each detector in a single logical expression, depending on which type of filtering is done.
Given their purpose, I'm not sure how to define the maps as const class members and initialize them in the class, and still be able to change their value when the filter checks track multiplicity conditions. Could you give an example of how to implement this?


// pixel map definition
pixel_filter_[CTPPSPixelDetId(0, 0, 3)] = false;
pixel_filter_[CTPPSPixelDetId(0, 2, 3)] = false;
pixel_filter_[CTPPSPixelDetId(1, 0, 3)] = false;
pixel_filter_[CTPPSPixelDetId(1, 2, 3)] = false;

Copy link
Contributor

Choose a reason for hiding this comment

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

pixel_filter never gets changed after these lines, so can be class members initialised in the class.
Same for the other two Boolean arrays.

// strip map definition
// diamond map definition

Copy link
Contributor

Choose a reason for hiding this comment

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

Why are there no strip/diam settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have the strip and diam options in place but not fully defined. This is done in case there is a future decision to include these detectors in the filter as well. As they are unused for the current implementation of the filter, all references to strip / diam conditions can be removed for clarity.

// First filter on pixels (2017+) selection
if (!pixel_filter_.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can pixel_filter_ still be empty here? It is filled right above

edm::Handle<edm::DetSetVector<CTPPSPixelLocalTrack>> pixelTracks;
iEvent.getByToken(pixelLocalTrackToken_, pixelTracks);

for (const auto& rpv : *pixelTracks) {
if (pixel_filter_.count(rpv.id) == 0) {
continue;
}
auto& fltr = pixel_filter_.at(rpv.id);
fltr = true;

const auto ntrks = std::count_if(rpv.begin(), rpv.end(), valid_trks);
if (minTracks_ > 0 && ntrks < minTracks_)
fltr = false;
if (maxTracks_ > 0 && ntrks > maxTracks_)
fltr = false;
}

// compilation of filter conditions
if (do_express_) {
return (pixel_filter_.at(CTPPSPixelDetId(0, 0, 3)) && pixel_filter_.at(CTPPSPixelDetId(0, 2, 3))) ||
(pixel_filter_.at(CTPPSPixelDetId(1, 0, 3)) && pixel_filter_.at(CTPPSPixelDetId(1, 2, 3)));
} else {
return (pixel_filter_.at(CTPPSPixelDetId(0, 0, 3)) || pixel_filter_.at(CTPPSPixelDetId(0, 2, 3))) ||
(pixel_filter_.at(CTPPSPixelDetId(1, 0, 3)) || pixel_filter_.at(CTPPSPixelDetId(1, 2, 3)));
}
}

// Then filter on strips (2016-17) selection
if (!strip_filter_.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

strip is always empty as it is not being set beforehand

edm::Handle<edm::DetSetVector<TotemRPLocalTrack>> stripTracks;
iEvent.getByToken(stripLocalTrackToken_, stripTracks);

for (const auto& rpv : *stripTracks) {
if (strip_filter_.count(rpv.id) == 0)
continue;
auto& fltr = strip_filter_.at(rpv.id);
fltr = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

If fltr is set to true always, then why the previous line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure what this comment is pointing out, but as mentioned in the previous reply, the strip / diam conditions can be removed, which would remove the problem for these detectors. Does this issue also apply to the case of the pixels? If so, please elaborate on what the issue is as I don't understand it at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

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

You set fltr to strip_filter something, and then the line immediatly afterwards you set fltr to true, so it gets reassigned a constant value...


const auto ntrks = std::count_if(rpv.begin(), rpv.end(), valid_trks);
if (minTracks_ > 0 && ntrks < minTracks_)
fltr = false;
if (maxTracks_ > 0 && ntrks > maxTracks_)
fltr = false;
}
}

// Finally filter on diamond (2016+) selection
if (!diam_filter_.empty()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

diam is always empty as it is not being set beforehand

edm::Handle<edm::DetSetVector<CTPPSDiamondLocalTrack>> diamondTracks;
iEvent.getByToken(diamondLocalTrackToken_, diamondTracks);

for (const auto& rpv : *diamondTracks) {
if (diam_filter_.count(rpv.id) == 0)
continue;
auto& fltr = diam_filter_.at(rpv.id);
fltr = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

If fltr is set to true always, then why the previous line?


const auto ntrks = std::count_if(rpv.begin(), rpv.end(), valid_trks);
if (minTracks_ > 0 && ntrks < minTracks_)
fltr = false;
if (maxTracks_ > 0 && ntrks > maxTracks_)
fltr = false;
}
}

return false;
}

// define as a framework module
#include "FWCore/Framework/interface/MakerMacros.h"
DEFINE_FWK_MODULE(HLTPPSCalFilter);