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

Restrict GEM/ME0 cluster size and max N clusters #19986

Merged
merged 6 commits into from Sep 1, 2017
Merged

Restrict GEM/ME0 cluster size and max N clusters #19986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6a0bf82
restrict cluster size and max n clusters
Aug 1, 2017
92e76de
fix indentation
Aug 1, 2017
ecef4bd
cleanup
Aug 1, 2017
ac4363e
fix seg fault
Aug 1, 2017
22eaf9e
Suggestion by David Lange
Aug 3, 2017
8b83ddb
send 8x8 clusters per ME0 layer
Aug 3, 2017
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
33 changes: 25 additions & 8 deletions SimMuon/GEMDigitizer/src/GEMPadDigiClusterProducer.cc
View file
Open in desktop
Expand Up @@ -53,10 +53,15 @@ void GEMPadDigiClusterProducer::produce(edm::Event& e, const edm::EventSetup& ev
}


void GEMPadDigiClusterProducer::buildClusters(const GEMPadDigiCollection &det_pads, GEMPadDigiClusterCollection &out_clusters)
void GEMPadDigiClusterProducer::buildClusters(const GEMPadDigiCollection &det_pads,
GEMPadDigiClusterCollection &out_clusters)
{
// construct clusters
for (const auto& ch: geometry_->chambers()) {
unsigned int nClusters = 0;

// proto collection
std::vector<std::pair<GEMDetId, GEMPadDigiCluster> > proto_clusters;
Copy link
Contributor

Choose a reason for hiding this comment

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

hi @dildick - could you not do the cleanup of extra digis (presumably not needed all the time..) directly in out_clusters instead of making this intermediate structure?

Copy link
Contributor Author

@dildick dildick Aug 9, 2017 •
edited

Choose a reason for hiding this comment

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

@davidlange6 The selection I implemented (maximum first 8 in the list) will need to be optimized for sure. Hardware tests will have to show how to best select GEM/ME0 clusters in realistic conditions. The final selection will likely be very different. With this setup we can simulate the effects of various selection procedures. So in due time we can modify the algorithm to not produce an intermediate proto_clusters

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @dildick - sorry for an extremely slow answer. I'd suggest we build this idea in from the beginning.I think its straightforward

  1. don't declare proto_clusters, just fill out_clusters.
  2. make a dedicated function for cleaning. -in the current case, its very trivial. But it could then easily grow to meet the more complex needs in the future. (and if you need to copy clusters at that point you could).

A different point - the way maxClusters_ is used below looks potentially incorrect. If it is really meant as the maximum size of what is now protoClusters, then don't let protoClusters grow beyond that size from the beginning. If instead its meant to be the maximum size of the final collection, then the loopmax concept is incorrect. Instead the out_clusters size needs to be checked at the end to see if it is too big.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Regarding point 1: could you point me to an example how to clean a muondigicollection such as out_clusters?

Copy link
Contributor

Choose a reason for hiding this comment

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

ah - sigh - i didn't realize that MuonDigiCollection exposes only a trivial bit of the interface that a standard vector has... i guess its not worth the trouble..

Copy link
Contributor Author

@dildick dildick Aug 29, 2017 •
edited

Choose a reason for hiding this comment

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

@davidlange6 what is the plan forward then? Can we go ahead with the current proposal?

Copy link
Contributor

Choose a reason for hiding this comment

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

hi @dildick - could you confirm the role of maxClusters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

max number of clusters (in the cluster digi collection) in a GEM chamber

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, as a folllowup pr, the check on max clusters can be applied directly when making proto_clusters


for (const auto& part: ch->etaPartitions()) {
auto pads = det_pads.get(part->id());
std::vector<uint16_t> cl;
Expand All @@ -66,24 +71,36 @@ void GEMPadDigiClusterProducer::buildClusters(const GEMPadDigiCollection &det_pa
cl.push_back((*d).pad());
}
else {
if ((*d).bx() == startBX and (*d).pad() == cl.back() + 1) {
if ((*d).bx() == startBX and // same bunch crossing
(*d).pad() == cl.back() + 1 // pad difference is 1
and cl.size()<maxClusterSize_) { // max 8 in cluster
cl.push_back((*d).pad());
}
else {
// put the current cluster in the proto collection
GEMPadDigiCluster pad_cluster(cl, startBX);
out_clusters.insertDigi(part->id(), pad_cluster);
proto_clusters.emplace_back(part->id(), pad_cluster);

// start a new cluster
cl.clear();
cl.push_back((*d).pad());
nClusters++;
}
}
startBX = (*d).bx();
}
// put the last cluster in the proto collection
if (pads.first != pads.second){
GEMPadDigiCluster pad_cluster(cl, startBX);
out_clusters.insertDigi(part->id(), pad_cluster);
nClusters++;
proto_clusters.emplace_back(part->id(), pad_cluster);
}
} // end of partition loop

// cluster selection: pick first maxClusters_ for now
unsigned loopMax=std::min(maxClusters_,unsigned(proto_clusters.size()));
for ( unsigned int i=0; i<loopMax; i++) {
const auto& detid(proto_clusters[i].first);
const auto& cluster(proto_clusters[i].second);
out_clusters.insertDigi(detid, cluster);
}
}
} // end of chamber loop
}
11 changes: 5 additions & 6 deletions SimMuon/GEMDigitizer/src/GEMPadDigiProducer.cc
View file
Open in desktop
Expand Up @@ -52,24 +52,23 @@ void GEMPadDigiProducer::produce(edm::Event& e, const edm::EventSetup& eventSetu

void GEMPadDigiProducer::buildPads(const GEMDigiCollection &det_digis, GEMPadDigiCollection &out_pads) const
{
auto etaPartitions = geometry_->etaPartitions();
for(const auto& p: etaPartitions)
for(const auto& p: geometry_->etaPartitions())
{
// set of <pad, bx> pairs, sorted first by pad then by bx
std::set<std::pair<int, int> > proto_pads;
// walk over digis in this partition,

// walk over digis in this partition,
// and stuff them into a set of unique pads (equivalent of OR operation)
auto digis = det_digis.get(p->id());
for (auto d = digis.first; d != digis.second; ++d)
{
int pad_num = 1 + static_cast<int>( p->padOfStrip(d->strip()) );
proto_pads.emplace(pad_num, d->bx());
}

// in the future, do some dead-time handling
// emulateDeadTime(proto_pads)

// fill the output collections
for (const auto& d: proto_pads)
{
Expand Down
32 changes: 26 additions & 6 deletions SimMuon/GEMDigitizer/src/ME0PadDigiClusterProducer.cc
View file
Open in desktop
Expand Up @@ -53,9 +53,15 @@ void ME0PadDigiClusterProducer::produce(edm::Event& e, const edm::EventSetup& ev
}


void ME0PadDigiClusterProducer::buildClusters(const ME0PadDigiCollection &det_pads, ME0PadDigiClusterCollection &out_clusters)
void ME0PadDigiClusterProducer::buildClusters(const ME0PadDigiCollection &det_pads,
ME0PadDigiClusterCollection &out_clusters)
{
for (const auto& ch: geometry_->chambers()) {
// construct clusters
for (const auto& ch: geometry_->layers()) {

// proto collection
std::vector<std::pair<ME0DetId, ME0PadDigiCluster> > proto_clusters;

for (const auto& part: ch->etaPartitions()) {
auto pads = det_pads.get(part->id());
std::vector<uint16_t> cl;
Expand All @@ -65,22 +71,36 @@ void ME0PadDigiClusterProducer::buildClusters(const ME0PadDigiCollection &det_pa
cl.push_back((*d).pad());
}
else {
if ((*d).bx() == startBX and (*d).pad() == cl.back() + 1) {
if ((*d).bx() == startBX and // same bunch crossing
(*d).pad() == cl.back() + 1 // pad difference is 1
and cl.size()<maxClusterSize_) { // max 8 in cluster
cl.push_back((*d).pad());
}
else {
// put the current cluster in the proto collection
ME0PadDigiCluster pad_cluster(cl, startBX);
out_clusters.insertDigi(part->id(), pad_cluster);
proto_clusters.emplace_back(part->id(), pad_cluster);

// start a new cluster
cl.clear();
cl.push_back((*d).pad());
}
}
startBX = (*d).bx();
}
// put the last cluster in the proto collection
if (pads.first != pads.second){
ME0PadDigiCluster pad_cluster(cl, startBX);
out_clusters.insertDigi(part->id(), pad_cluster);
proto_clusters.emplace_back(part->id(), pad_cluster);
}
} // end of partition loop

// cluster selection: pick first maxClusters_ for now
unsigned loopMax=std::min(maxClusters_,unsigned(proto_clusters.size()));
for ( unsigned int i=0; i<loopMax; i++) {
const auto& detid(proto_clusters[i].first);
const auto& cluster(proto_clusters[i].second);
out_clusters.insertDigi(detid, cluster);
}
}
} // end of chamber loop
}
3 changes: 1 addition & 2 deletions SimMuon/GEMDigitizer/src/ME0PadDigiProducer.cc
View file
Open in desktop
Expand Up @@ -51,8 +51,7 @@ void ME0PadDigiProducer::produce(edm::Event& e, const edm::EventSetup& eventSetu

void ME0PadDigiProducer::buildPads(const ME0DigiCollection &det_digis, ME0PadDigiCollection &out_pads) const
{
auto etaPartitions = geometry_->etaPartitions();
for(const auto& p: etaPartitions)
for(const auto& p: geometry_->etaPartitions())
{
// set of <pad, bx> pairs, sorted first by pad then by bx
std::set<std::pair<int, int> > proto_pads;
Expand Down