Skip to content

Commit

Permalink
fix!: Fix vertex finding for seeds with z=0 (#2917)
Browse files Browse the repository at this point in the history
While working on the 4D vertexing performance I realized that our vertex finders will abort if a seed with `z=0` is found. This can happen using a grid based seeder as it can round to exactly 0.

As far as I could see the vertex constraint was only used to signal the finish line for the vertex finding. I exchanged that with an optional and also removed the vector which had no use as far as I could tell
  • Loading branch information
andiwand authored Feb 21, 2024
1 parent 8ccefe1 commit 123746c
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 64 deletions.
Binary file modified CI/physmon/reference/performance_ivf_truth_estimated_hist.root
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,9 @@ inline auto Acts::AdaptiveGridDensityVertexFinder::find(

if (state.mainDensityMap.empty()) {
// No tracks passed selection
// Return empty seed, i.e. vertex at constraint position
// Return empty seed
// (Note: Upstream finder should check for this break condition)
std::vector<Vertex> seedVec{vertexingOptions.constraint};
return seedVec;
return std::vector<Vertex>{};
}

double z = 0;
Expand Down Expand Up @@ -90,9 +89,7 @@ inline auto Acts::AdaptiveGridDensityVertexFinder::find(

returnVertex.setFullCovariance(seedCov);

std::vector<Vertex> seedVec{returnVertex};

return seedVec;
return std::vector<Vertex>{returnVertex};
}

inline auto Acts::AdaptiveGridDensityVertexFinder::doesPassTrackSelection(
Expand Down
2 changes: 1 addition & 1 deletion Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class AdaptiveMultiVertexFinder final : public IVertexFinder {
/// from seed track collection in last iteration
///
/// @return The seed vertex
Result<Vertex> doSeeding(
Result<std::optional<Vertex>> doSeeding(
const std::vector<InputTrack>& trackVector, Vertex& currentConstraint,
const VertexingOptions& vertexingOptions,
IVertexFinder::State& seedFinderState,
Expand Down
50 changes: 30 additions & 20 deletions Core/include/Acts/Vertexing/AdaptiveMultiVertexFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -38,41 +38,43 @@ inline auto Acts::AdaptiveMultiVertexFinder::find(
std::vector<InputTrack> removedSeedTracks;
while (!seedTracks.empty() && iteration < m_cfg.maxIterations &&
(m_cfg.addSingleTrackVertices || seedTracks.size() >= 2)) {
// Tracks that are used for searching compatible tracks
// near a vertex candidate
std::vector<InputTrack> searchTracks;
if (m_cfg.doRealMultiVertex) {
searchTracks = origTracks;
} else {
searchTracks = seedTracks;
}
Vertex currentConstraint = vertexingOptions.constraint;

// Retrieve seed vertex from all remaining seedTracks
auto seedResult = doSeeding(seedTracks, currentConstraint, vertexingOptions,
seedFinderState, removedSeedTracks);
if (!seedResult.ok()) {
return seedResult.error();
}
allVertices.push_back(std::make_unique<Vertex>(*seedResult));
const auto& seedOptional = *seedResult;

Vertex& vtxCandidate = *allVertices.back();
allVerticesPtr.push_back(&vtxCandidate);

ACTS_DEBUG("Position of vertex candidate after seeding: "
<< vtxCandidate.fullPosition().transpose());
if (vtxCandidate.position().z() ==
vertexingOptions.constraint.position().z()) {
if (!seedOptional.has_value()) {
ACTS_DEBUG(
"No seed found anymore. Break and stop primary vertex finding.");
allVertices.pop_back();
allVerticesPtr.pop_back();
break;
}
const auto& seedVertex = seedOptional.value();

ACTS_DEBUG("Position of vertex candidate after seeding: "
<< seedVertex.fullPosition().transpose());

allVertices.push_back(std::make_unique<Vertex>(seedVertex));
Vertex& vtxCandidate = *allVertices.back();
allVerticesPtr.push_back(&vtxCandidate);

// Clear the seed track collection that has been removed in last iteration
// now after seed finding is done
removedSeedTracks.clear();

// Tracks that are used for searching compatible tracks near a vertex
// candidate
std::vector<InputTrack> searchTracks;
if (m_cfg.doRealMultiVertex) {
searchTracks = origTracks;
} else {
searchTracks = seedTracks;
}

auto prepResult = canPrepareVertexForFit(searchTracks, seedTracks,
vtxCandidate, currentConstraint,
fitterState, vertexingOptions);
Expand Down Expand Up @@ -144,7 +146,8 @@ inline auto Acts::AdaptiveMultiVertexFinder::doSeeding(
const std::vector<InputTrack>& trackVector, Vertex& currentConstraint,
const VertexingOptions& vertexingOptions,
IVertexFinder::State& seedFinderState,
const std::vector<InputTrack>& removedSeedTracks) const -> Result<Vertex> {
const std::vector<InputTrack>& removedSeedTracks) const
-> Result<std::optional<Vertex>> {
VertexingOptions seedOptions = vertexingOptions;
seedOptions.constraint = currentConstraint;

Expand All @@ -157,8 +160,15 @@ inline auto Acts::AdaptiveMultiVertexFinder::doSeeding(
if (!seedResult.ok()) {
return seedResult.error();
}
const auto& seedVector = *seedResult;

ACTS_DEBUG("Found " << seedVector.size() << " seeds");

if (seedVector.empty()) {
return std::nullopt;
}
Vertex seedVertex = seedVector.back();

Vertex seedVertex = (*seedResult).back();
// Update constraints according to seed vertex
setConstraintAfterSeeding(currentConstraint, seedOptions.useConstraintInFit,
seedVertex);
Expand Down
9 changes: 3 additions & 6 deletions Core/include/Acts/Vertexing/GridDensityVertexFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ auto Acts::GridDensityVertexFinder<mainGridSize, trkGridSize>::find(
}
if (!couldRemoveTracks) {
// No tracks were removed anymore
// Return empty seed, i.e. vertex at constraint position
// Return empty seed
// (Note: Upstream finder should check for this break condition)
std::vector<Vertex> seedVec{vertexingOptions.constraint};
return seedVec;
return std::vector<Vertex>{};
}
} else {
state.mainGrid = MainGridVector::Zero();
Expand Down Expand Up @@ -94,9 +93,7 @@ auto Acts::GridDensityVertexFinder<mainGridSize, trkGridSize>::find(

returnVertex.setFullCovariance(seedCov);

std::vector<Vertex> seedVec{returnVertex};

return seedVec;
return std::vector<Vertex>{returnVertex};
}

template <int mainGridSize, int trkGridSize>
Expand Down
11 changes: 6 additions & 5 deletions Core/include/Acts/Vertexing/IterativeVertexFinder.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,13 @@
#include "Acts/MagneticField/MagneticFieldProvider.hpp"
#include "Acts/Utilities/Logger.hpp"
#include "Acts/Utilities/Result.hpp"
#include "Acts/Vertexing/FsmwMode1dFinder.hpp"
#include "Acts/Vertexing/FullBilloirVertexFitter.hpp"
#include "Acts/Vertexing/HelicalTrackLinearizer.hpp"
#include "Acts/Vertexing/IVertexFinder.hpp"
#include "Acts/Vertexing/ImpactPointEstimator.hpp"
#include "Acts/Vertexing/TrackLinearizer.hpp"
#include "Acts/Vertexing/Vertex.hpp"
#include "Acts/Vertexing/VertexingOptions.hpp"
#include "Acts/Vertexing/ZScanVertexFinder.hpp"

#include <functional>

Expand Down Expand Up @@ -203,11 +201,14 @@ class IterativeVertexFinder final : public IVertexFinder {

/// @brief Method that calls seed finder to retrieve a vertex seed
///
/// @param state The state object
/// @param seedTracks Seeding tracks
/// @param vertexingOptions Vertexing options
Result<Vertex> getVertexSeed(State& state,
const std::vector<InputTrack>& seedTracks,
const VertexingOptions& vertexingOptions) const;
///
/// @return Vertex seed
Result<std::optional<Vertex>> getVertexSeed(
State& state, const std::vector<InputTrack>& seedTracks,
const VertexingOptions& vertexingOptions) const;

/// @brief Removes all tracks in tracksToRemove from seedTracks
///
Expand Down
26 changes: 10 additions & 16 deletions Core/include/Acts/Vertexing/IterativeVertexFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,13 @@ inline auto Acts::IterativeVertexFinder::find(
if (!seedRes.ok()) {
return seedRes.error();
}
const auto& seedOptional = *seedRes;

const auto& seedVertex = *seedRes;

if (seedVertex.fullPosition()[eZ] ==
vertexingOptions.constraint.position().z()) {
if (!seedOptional.has_value()) {
ACTS_DEBUG("No more seed found. Break and stop primary vertex finding.");
break;
}
const auto& seedVertex = *seedOptional;

/// End seeding
/// Now take only tracks compatible with current seed
Expand Down Expand Up @@ -156,7 +155,8 @@ inline auto Acts::IterativeVertexFinder::find(

inline auto Acts::IterativeVertexFinder::getVertexSeed(
State& state, const std::vector<InputTrack>& seedTracks,
const VertexingOptions& vertexingOptions) const -> Result<Vertex> {
const VertexingOptions& vertexingOptions) const
-> Result<std::optional<Vertex>> {
auto finderState = m_cfg.seedFinder->makeState(state.magContext);
auto res = m_cfg.seedFinder->find(seedTracks, vertexingOptions, finderState);

Expand All @@ -165,20 +165,14 @@ inline auto Acts::IterativeVertexFinder::getVertexSeed(
<< seedTracks.size());
return VertexingError::SeedingError;
}
const auto& seedVector = *res;

const auto& vertexCollection = *res;
ACTS_DEBUG("Found " << seedVector.size() << " seeds");

if (vertexCollection.empty()) {
ACTS_ERROR("Empty seed collection was returned. Number of input tracks: "
<< seedTracks.size());
return VertexingError::SeedingError;
if (seedVector.empty()) {
return std::nullopt;
}

ACTS_DEBUG("Found " << vertexCollection.size() << " seeds");

// retrieve the seed vertex as the last element in
// the seed vertexCollection
Vertex seedVertex = vertexCollection.back();
const Vertex& seedVertex = seedVector.back();

ACTS_DEBUG("Use " << seedTracks.size() << " tracks for vertex seed finding.")
ACTS_DEBUG(
Expand Down
4 changes: 1 addition & 3 deletions Core/include/Acts/Vertexing/TrackDensityVertexFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,5 @@ inline auto Acts::TrackDensityVertexFinder::find(

returnVertex.setFullCovariance(seedCov);

std::vector<Vertex> seedVec{returnVertex};

return seedVec;
return std::vector<Vertex>{returnVertex};
}
8 changes: 1 addition & 7 deletions Core/include/Acts/Vertexing/ZScanVertexFinder.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,5 @@ inline auto Acts::ZScanVertexFinder::find(
vertexingOptions.constraint.time());
Vertex vtxResult = Vertex(output);

// Vector to be filled with one single vertex
std::vector<Vertex> vertexCollection;

// Add vertex to vertexCollection
vertexCollection.push_back(vtxResult);

return vertexCollection;
return std::vector<Vertex>{vtxResult};
}

0 comments on commit 123746c

Please sign in to comment.