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

Fix crash in PF Clustering due to empty HCAL RecHits collection #44693

Merged
merged 3 commits into from
Apr 11, 2024
Merged

Fix crash in PF Clustering due to empty HCAL RecHits collection #44693

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
618f781
Fix PF crash due to empty HCAL RecHits collection
jsamudio Apr 9, 2024
141e0b4
Formatting changes
jsamudio Apr 10, 2024
0398bdc
Simplify size checks
jsamudio Apr 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 29 additions & 25 deletions RecoParticleFlow/PFClusterProducer/plugins/LegacyPFClusterProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,41 +197,45 @@ void LegacyPFClusterProducer::produce(edm::Event& event, const edm::EventSetup&
auto const& pfClusterSoA = event.get(pfClusterSoAToken_).const_view();
auto const& pfRecHitFractionSoA = event.get(pfRecHitFractionSoAToken_).const_view();

int nRH = pfRecHits.view().size();
int nRH = 0;
if (pfRecHits->metadata().size() != 0)
nRH = pfRecHits.view().size();
reco::PFClusterCollection out;
out.reserve(nRH);

auto const rechitsHandle = event.getHandle(recHitsLabel_);

// Build PFClusters in legacy format
std::vector<int> nTopoSeeds(nRH, 0);
if (nRH != 0) {
// Build PFClusters in legacy format
std::vector<int> nTopoSeeds(nRH, 0);

for (int i = 0; i < pfClusterSoA.nSeeds(); i++) {
nTopoSeeds[pfClusterSoA[i].topoId()]++;
}
for (int i = 0; i < pfClusterSoA.nSeeds(); i++) {
nTopoSeeds[pfClusterSoA[i].topoId()]++;
}

// Looping over SoA PFClusters to produce legacy PFCluster collection
for (int i = 0; i < pfClusterSoA.nSeeds(); i++) {
unsigned int n = pfClusterSoA[i].seedRHIdx();
reco::PFCluster temp;
temp.setSeed((*rechitsHandle)[n].detId()); // Pulling the detId of this PFRecHit from the legacy format input
int offset = pfClusterSoA[i].rhfracOffset();
for (int k = offset; k < (offset + pfClusterSoA[i].rhfracSize()) && k >= 0;
k++) { // Looping over PFRecHits in the same topo cluster
if (pfRecHitFractionSoA[k].pfrhIdx() < nRH && pfRecHitFractionSoA[k].pfrhIdx() > -1 &&
pfRecHitFractionSoA[k].frac() > 0.0) {
const reco::PFRecHitRef& refhit = reco::PFRecHitRef(rechitsHandle, pfRecHitFractionSoA[k].pfrhIdx());
temp.addRecHitFraction(reco::PFRecHitFraction(refhit, pfRecHitFractionSoA[k].frac()));
// Looping over SoA PFClusters to produce legacy PFCluster collection
for (int i = 0; i < pfClusterSoA.nSeeds(); i++) {
unsigned int n = pfClusterSoA[i].seedRHIdx();
reco::PFCluster temp;
temp.setSeed((*rechitsHandle)[n].detId()); // Pulling the detId of this PFRecHit from the legacy format input
int offset = pfClusterSoA[i].rhfracOffset();
for (int k = offset; k < (offset + pfClusterSoA[i].rhfracSize()) && k >= 0;
k++) { // Looping over PFRecHits in the same topo cluster
if (pfRecHitFractionSoA[k].pfrhIdx() < nRH && pfRecHitFractionSoA[k].pfrhIdx() > -1 &&
pfRecHitFractionSoA[k].frac() > 0.0) {
const reco::PFRecHitRef& refhit = reco::PFRecHitRef(rechitsHandle, pfRecHitFractionSoA[k].pfrhIdx());
temp.addRecHitFraction(reco::PFRecHitFraction(refhit, pfRecHitFractionSoA[k].frac()));
}
}
}

// Now PFRecHitFraction of this PFCluster is set. Now compute calculateAndSetPosition (energy, position etc)
if (nTopoSeeds[pfClusterSoA[i].topoId()] == 1 && allCellsPositionCalc_) {
allCellsPositionCalc_->calculateAndSetPosition(temp, paramPF);
} else {
positionCalc_->calculateAndSetPosition(temp, paramPF);
// Now PFRecHitFraction of this PFCluster is set. Now compute calculateAndSetPosition (energy, position etc)
if (nTopoSeeds[pfClusterSoA[i].topoId()] == 1 && allCellsPositionCalc_) {
allCellsPositionCalc_->calculateAndSetPosition(temp, paramPF);
} else {
positionCalc_->calculateAndSetPosition(temp, paramPF);
}
out.emplace_back(std::move(temp));
}
out.emplace_back(std::move(temp));
}

event.emplace(legacyPfClustersToken_, std::move(out));
Expand Down
18 changes: 14 additions & 4 deletions RecoParticleFlow/PFClusterProducer/plugins/alpaka/PFClusterSoAProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,26 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {
const reco::PFClusterParamsDeviceCollection& params = setup.getData(pfClusParamsToken);
const reco::PFRecHitHCALTopologyDeviceCollection& topology = setup.getData(topologyToken_);
const reco::PFRecHitHostCollection& pfRecHits = event.get(inputPFRecHitSoA_Token_);
const int nRH = pfRecHits->size();
int nRH = 0;
if (pfRecHits->metadata().size() != 0)
nRH = pfRecHits->size();

reco::PFClusteringVarsDeviceCollection pfClusteringVars{nRH, event.queue()};
reco::PFClusteringEdgeVarsDeviceCollection pfClusteringEdgeVars{(nRH * 8), event.queue()};
reco::PFClusterDeviceCollection pfClusters{nRH, event.queue()};
reco::PFRecHitFractionDeviceCollection pfrhFractions{nRH * pfRecHitFractionAllocation_, event.queue()};

PFClusterProducerKernel kernel(event.queue(), pfRecHits);
kernel.execute(
event.queue(), params, topology, pfClusteringVars, pfClusteringEdgeVars, pfRecHits, pfClusters, pfrhFractions);
if (nRH != 0) {
PFClusterProducerKernel kernel(event.queue(), pfRecHits);
kernel.execute(event.queue(),
params,
topology,
pfClusteringVars,
pfClusteringEdgeVars,
pfRecHits,
pfClusters,
pfrhFractions);
}

if (synchronise_)
alpaka::wait(event.queue());
Expand Down
35 changes: 20 additions & 15 deletions RecoParticleFlow/PFRecHitProducer/plugins/LegacyPFRecHitProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,23 +54,28 @@ void LegacyPFRecHitProducer::produce(edm::Event& event, const edm::EventSetup& s
const reco::PFRecHitHostCollection::ConstView& alpakaPfRecHits = pfRecHitsAlpakaSoA.const_view();

reco::PFRecHitCollection out;
out.reserve(alpakaPfRecHits.size());
int reserveSize = 0;
if (alpakaPfRecHits.metadata().size() != 0)
reserveSize = alpakaPfRecHits.size();
out.reserve(reserveSize);

for (size_t i = 0; i < alpakaPfRecHits.size(); i++) {
reco::PFRecHit& pfrh =
out.emplace_back(caloGeo_.at(alpakaPfRecHits[i].layer())->getGeometry(alpakaPfRecHits[i].detId()),
alpakaPfRecHits[i].detId(),
alpakaPfRecHits[i].layer(),
alpakaPfRecHits[i].energy());
pfrh.setTime(alpakaPfRecHits[i].time());
pfrh.setDepth(alpakaPfRecHits[i].depth());
if (alpakaPfRecHits.metadata().size() != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this could be more simply

reco::PFRecHitCollection out;

if (alpakaPfRecHits.metadata().size() != 0) {
  out.reserve(alpakaPfRecHits.size());
  for (size_t i = 0; i < alpakaPfRecHits.size(); i++) {
    ...

for (size_t i = 0; i < alpakaPfRecHits.size(); i++) {
reco::PFRecHit& pfrh =
out.emplace_back(caloGeo_.at(alpakaPfRecHits[i].layer())->getGeometry(alpakaPfRecHits[i].detId()),
alpakaPfRecHits[i].detId(),
alpakaPfRecHits[i].layer(),
alpakaPfRecHits[i].energy());
pfrh.setTime(alpakaPfRecHits[i].time());
pfrh.setDepth(alpakaPfRecHits[i].depth());

// order in Alpaka: N, S, E, W,NE,SW,SE,NW
const short eta[8] = {0, 0, 1, -1, 1, -1, 1, -1};
const short phi[8] = {1, -1, 0, 0, 1, -1, -1, 1};
for (size_t k = 0; k < 8; k++)
if (alpakaPfRecHits[i].neighbours()(k) != -1)
pfrh.addNeighbour(eta[k], phi[k], 0, alpakaPfRecHits[i].neighbours()(k));
// order in Alpaka: N, S, E, W,NE,SW,SE,NW
const short eta[8] = {0, 0, 1, -1, 1, -1, 1, -1};
const short phi[8] = {1, -1, 0, 0, 1, -1, -1, 1};
for (size_t k = 0; k < 8; k++)
if (alpakaPfRecHits[i].neighbours()(k) != -1)
pfrh.addNeighbour(eta[k], phi[k], 0, alpakaPfRecHits[i].neighbours()(k));
}
}

event.emplace(legacyPfRecHitsToken_, out);
Expand Down
11 changes: 7 additions & 4 deletions RecoParticleFlow/PFRecHitProducer/plugins/alpaka/PFRecHitSoAProducer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ namespace ALPAKA_ACCELERATOR_NAMESPACE {

reco::PFRecHitDeviceCollection pfRecHits{(int)num_recHits, event.queue()};

PFRecHitProducerKernel<CAL> kernel{event.queue(), num_recHits};
for (const auto& token : recHitsToken_)
kernel.processRecHits(event.queue(), event.get(token.first), setup.getData(token.second), topology, pfRecHits);
kernel.associateTopologyInfo(event.queue(), topology, pfRecHits);
if (pfRecHits->metadata().size() != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (pfRecHits->metadata().size() != 0) {
if (num_recHits != 0) {

?

PFRecHitProducerKernel<CAL> kernel{event.queue(), num_recHits};
for (const auto& token : recHitsToken_)
kernel.processRecHits(
event.queue(), event.get(token.first), setup.getData(token.second), topology, pfRecHits);
kernel.associateTopologyInfo(event.queue(), topology, pfRecHits);
}

if (synchronise_)
alpaka::wait(event.queue());
Expand Down