Skip to content

Commit

Permalink
fix: small bug when avoiding iterations while looking for dublets (#2304
Browse files Browse the repository at this point in the history
)

While looping over the doublets, we find the first SP inside the radius region of interest and update the iterator so we don't need to look at the other SPs again. That mean that we have changed the middle space point (which are sorted in radius) and the radius lower bound has moved accordingly.

However, if that is the last bottom or top spaceport in that grid bin, we continue and we don't move the lower bound. This means that we will continue looking at some spacepoints unnecessarily.
This PR is intended to fix that and update the lower bound also for these cases.

@CarloVarni @noemina let me know if you have a better suggestion on how to fix this

I tested this and improves slightly ~4% performance for ttbar 200 pileUp
  • Loading branch information
LuisFelipeCoelho committed Jul 14, 2023
1 parent f1bc93a commit 8fbbc46
Showing 1 changed file with 21 additions and 18 deletions.
39 changes: 21 additions & 18 deletions Core/include/Acts/Seeding/SeedFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,27 @@ SeedFinder<external_spacepoint_t, platform_t>::getCompatibleDoublets(
// we make a copy of the iterator here since we need it to remain
// the same in the Neighbour object
auto min_itr = otherSPCol.itr;
bool found = false;

// find the first SP inside the radius region of interest and update
// the iterator so we don't need to look at the other SPs again
for (; min_itr != otherSPs.end(); ++min_itr) {
const auto& otherSP = *min_itr;
if constexpr (candidateType == Acts::SpacePointCandidateType::eBottom) {
// if r-distance is too big, try next SP in bin
if ((rM - otherSP->radius()) <= deltaRMaxSP) {
break;
}
} else {
// if r-distance is too small, try next SP in bin
if ((otherSP->radius() - rM) >= deltaRMinSP) {
break;
}
}
}
// We update the iterator in the Neighbour object
// that mean that we have changed the middle space point
// and the lower bound has moved accordingly
otherSPCol.itr = min_itr;

for (; min_itr != otherSPs.end(); ++min_itr) {
const auto& otherSP = *min_itr;
Expand All @@ -254,30 +274,13 @@ SeedFinder<external_spacepoint_t, platform_t>::getCompatibleDoublets(
if (deltaR < deltaRMinSP) {
break;
}
// if r-distance is too big, try next SP in bin
if (deltaR > deltaRMaxSP) {
continue;
}

} else {
deltaR = (otherSP->radius() - rM);

// if r-distance is too big, try next SP in bin
if (deltaR > deltaRMaxSP) {
break;
}
// if r-distance is too small, try next SP in bin
if (deltaR < deltaRMinSP) {
continue;
}
}

// We update the iterator in the Neighbour object
// that mean that we have changed the middle space point
// and the lower bound has moved accordingly
if (not found) {
found = true;
otherSPCol.itr = min_itr;
}

if constexpr (candidateType == Acts::SpacePointCandidateType::eBottom) {
Expand Down

0 comments on commit 8fbbc46

Please sign in to comment.