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

fix: Return nullptr if outside tracking geometry in TrackingGeometry::lowestTrackingVolume #3481

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
01541b3
fix: Return `nullptr` if outside tracking geometry in `TrackingGeomet…
andiwand Aug 6, 2024
8994c9a
make volume bigger
andiwand Aug 6, 2024
9617e7f
more fixes
andiwand Aug 6, 2024
d41a4e7
fix navigation initialization with nullptr start volume
andiwand Aug 6, 2024
1cb5443
refactor gx2f start volume check
andiwand Aug 6, 2024
cef9ab8
Merge branch 'main' of github.com:acts-project/acts into fix-lowestTr…
andiwand Aug 6, 2024
93f54b5
update refs
andiwand Aug 6, 2024
40ad679
revert volume size
andiwand Aug 6, 2024
74e5148
Merge branch 'main' of github.com:acts-project/acts into fix-lowestTr…
andiwand Aug 9, 2024
3017044
Merge branch 'main' into fix-lowestTrackingVolume-allow-nullptr
andiwand Aug 22, 2024
957543d
Merge branch 'main' into fix-lowestTrackingVolume-allow-nullptr
andiwand Aug 25, 2024
b798228
Merge branch 'main' into fix-lowestTrackingVolume-allow-nullptr
andiwand Aug 28, 2024
fedbb3d
update refs
andiwand Aug 28, 2024
c9e0bb0
Merge branch 'main' of github.com:acts-project/acts into fix-lowestTr…
andiwand Aug 29, 2024
54e9f72
Merge branch 'main' into fix-lowestTrackingVolume-allow-nullptr
andiwand Aug 29, 2024
9b279c6
revert gx2f changes
andiwand Aug 30, 2024
63785af
Merge branch 'main' of github.com:acts-project/acts into fix-lowestTr…
andiwand Aug 30, 2024
a7d7731
fix gx2f again
andiwand Aug 30, 2024
2404ccf
refactor: Remove GX2F start volume checks
andiwand Aug 30, 2024
48e1a08
Merge branch 'remove-gx2f-start-volume-checks' of github.com:andiwand…
andiwand Aug 30, 2024
7cb247e
Merge branch 'main' of github.com:acts-project/acts into fix-lowestTr…
andiwand Aug 31, 2024
581d5b7
update refs
andiwand Aug 31, 2024
40dea89
Merge branch 'main' into fix-lowestTrackingVolume-allow-nullptr
andiwand Aug 31, 2024
fab7721
revert gsf tests
andiwand Aug 31, 2024
ac03008
Merge branch 'fix-lowestTrackingVolume-allow-nullptr' of github.com:a…
andiwand Aug 31, 2024
6441203
revert gx2f test
andiwand Aug 31, 2024
5dd2516
Merge branch 'main' into fix-lowestTrackingVolume-allow-nullptr
kodiakhq[bot] Aug 31, 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
Binary file modified CI/physmon/reference/performance_ambi_ttbar.root
Binary file not shown.
Binary file modified CI/physmon/reference/performance_amvf_gridseeder_ttbar_hist.root
Binary file not shown.
Binary file modified CI/physmon/reference/performance_amvf_ttbar_hist.root
Binary file not shown.
Binary file modified CI/physmon/reference/performance_ckf_ttbar.root
Binary file not shown.
Binary file modified CI/physmon/reference/performance_gsf.root
Binary file not shown.
Binary file modified CI/physmon/reference/tracksummary_ckf_ttbar_hist.root
Binary file not shown.
2 changes: 1 addition & 1 deletion Core/include/Acts/Geometry/TrackingVolume.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ class TrackingVolume : public Volume {
/// static layers
std::unique_ptr<const LayerArray> m_confinedLayers = nullptr;

/// Array of Volumes inside the Volume when actin as container
/// Array of Volumes inside the Volume when acting as container
std::shared_ptr<const TrackingVolumeArray> m_confinedVolumes = nullptr;

/// confined dense
Expand Down
5 changes: 5 additions & 0 deletions Core/include/Acts/Propagator/Navigator.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,11 @@ class Navigator {
: nullptr;
if (state.navigation.startVolume != nullptr) {
ACTS_VERBOSE(volInfo(state) << "Start volume resolved.");
} else {
ACTS_VERBOSE(volInfo(state)
<< "No start volume resolved. Nothing left to do.");
// set the navigation break
state.navigation.navigationBreak = true;
}
}

Expand Down
32 changes: 19 additions & 13 deletions Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,25 @@ class Gx2Fitter {
const Logger& /*logger*/) const {
assert(result.fittedStates && "No MultiTrajectory set");

// Sanity check in the pre propagation phase
if (state.stage == PropagatorStage::prePropagation) {
// Check if we are in the expected volume
if (startVolume != nullptr &&
startVolume != state.navigation.startVolume) {
ACTS_INFO("The update pushed us to a new volume from '"
<< startVolume->volumeName() << "' to '"
<< ((state.navigation.startVolume != nullptr)
? state.navigation.startVolume->volumeName()
: "nullptr")
<< "'. Starting to abort.");
result.result =
Result<void>(Experimental::GlobalChiSquareFitterError::
UpdatePushedToNewVolume);
return;
}
result.startVolume = state.navigation.startVolume;
}

// Check if we can stop to propagate
if (result.measurementStates == inputMeasurements->size()) {
ACTS_INFO("Actor: finish: All measurements have been found.");
Expand All @@ -442,19 +461,6 @@ class Gx2Fitter {
return;
}

if (startVolume != nullptr &&
startVolume != state.navigation.startVolume) {
ACTS_INFO("The update pushed us to a new volume from '"
<< startVolume->volumeName() << "' to '"
<< state.navigation.startVolume->volumeName()
<< "'. Starting to abort.");
result.result = Result<void>(
Experimental::GlobalChiSquareFitterError::UpdatePushedToNewVolume);

return;
}
result.startVolume = state.navigation.startVolume;

// Update:
// - Waiting for a current surface
auto surface = navigator.currentSurface(state.navigation);
Expand Down
13 changes: 5 additions & 8 deletions Core/src/Geometry/TrackingGeometry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,7 @@ Acts::TrackingGeometry::~TrackingGeometry() = default;

const Acts::TrackingVolume* Acts::TrackingGeometry::lowestTrackingVolume(
const GeometryContext& gctx, const Acts::Vector3& gp) const {
const TrackingVolume* searchVolume = m_world.get();
const TrackingVolume* currentVolume = nullptr;
while (currentVolume != searchVolume && (searchVolume != nullptr)) {
currentVolume = searchVolume;
searchVolume = searchVolume->lowestTrackingVolume(gctx, gp);
}
return currentVolume;
return m_world->lowestTrackingVolume(gctx, gp);
}

const Acts::TrackingVolume* Acts::TrackingGeometry::highestTrackingVolume()
Expand All @@ -63,7 +57,10 @@ Acts::TrackingGeometry::highestTrackingVolumeShared() const {

const Acts::Layer* Acts::TrackingGeometry::associatedLayer(
const GeometryContext& gctx, const Acts::Vector3& gp) const {
const TrackingVolume* lowestVol = (lowestTrackingVolume(gctx, gp));
const TrackingVolume* lowestVol = lowestTrackingVolume(gctx, gp);
if (lowestVol == nullptr) {
return nullptr;
}
return lowestVol->associatedLayer(gctx, gp);
}

Expand Down
11 changes: 9 additions & 2 deletions Core/src/Geometry/TrackingVolume.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,18 @@ TrackingVolume::TrackingVolume(const Transform3& transform,
TrackingVolume::~TrackingVolume() = default;

const TrackingVolume* TrackingVolume::lowestTrackingVolume(
const GeometryContext& /*gctx*/, const Vector3& position,
const GeometryContext& gctx, const Vector3& position,
const double tol) const {
if (!inside(position, tol)) {
return nullptr;
}

// confined static volumes - highest hierarchy
if (m_confinedVolumes) {
return (m_confinedVolumes->object(position).get());
const TrackingVolume* volume = m_confinedVolumes->object(position).get();
if (volume != nullptr) {
return volume->lowestTrackingVolume(gctx, position, tol);
}
}

// search for dense volumes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,19 @@ struct SensitiveCandidates {
/// @return a vector of sensitive surfaces
std::vector<const Acts::Surface*> operator()(
const Acts::GeometryContext& gctx, const Acts::Vector3& position) const {
std::vector<const Acts::Surface*> surfaces;
if (trackingGeometry == nullptr) {
return {};
}

if (trackingGeometry != nullptr) {
auto layer = trackingGeometry->associatedLayer(gctx, position);
auto layer = trackingGeometry->associatedLayer(gctx, position);
if (layer == nullptr || layer->surfaceArray() == nullptr) {
return {};
}

if (layer->surfaceArray() != nullptr) {
for (const auto& surface : layer->surfaceArray()->surfaces()) {
if (surface->associatedDetectorElement() != nullptr) {
surfaces.push_back(surface);
}
}
std::vector<const Acts::Surface*> surfaces;
for (const auto& surface : layer->surfaceArray()->surfaces()) {
if (surface->associatedDetectorElement() != nullptr) {
surfaces.push_back(surface);
}
}
return surfaces;
Expand Down
7 changes: 5 additions & 2 deletions Tests/UnitTests/Core/Geometry/CuboidVolumeBuilderTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,16 @@ BOOST_AUTO_TEST_CASE(CuboidVolumeBuilderTest) {
std::unique_ptr<const TrackingGeometry> detector =
tgb.trackingGeometry(tgContext);
BOOST_CHECK_EQUAL(
detector->lowestTrackingVolume(tgContext, Vector3(1., 0., 0.))
detector->lowestTrackingVolume(tgContext, Vector3(1_mm, 0_mm, 0_mm))
->volumeName(),
volumeConfig.name);
BOOST_CHECK_EQUAL(
detector->lowestTrackingVolume(tgContext, Vector3(-1., 0., 0.))
detector->lowestTrackingVolume(tgContext, Vector3(-1_mm, 0_mm, 0_mm))
->volumeName(),
volumeConfig2.name);
BOOST_CHECK_EQUAL(
detector->lowestTrackingVolume(tgContext, Vector3(1000_m, 0_m, 0_m)),
nullptr);
}

} // namespace Acts::Test
4 changes: 1 addition & 3 deletions Tests/UnitTests/Core/TrackFitting/Gx2fTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,16 +291,14 @@ std::shared_ptr<const TrackingGeometry> makeToyDetectorYdirection(
// volumeConfig2.length = volumeConfig.length;
volumeConfig2.length = {2 * halfSizeSurface, (nSurfaces + 1) * 1_m,
2 * halfSizeSurface};
;
volumeConfig2.position = {volumeConfig2.length.x(),
volumeConfig2.length.y() / 2, 0.};
volumeConfig2.name = "AdditionalVolume";
volumeConfig2.binningDimension = Acts::BinningValue::binY;

// Outer volume - Build TrackingGeometry configuration and fill
CuboidVolumeBuilder::Config config;
config.length = {4 * halfSizeSurface, (nSurfaces + 1) * 1_m,
2 * halfSizeSurface};
config.length = {10_m, 10_m, 10_m};
andiwand marked this conversation as resolved.
Show resolved Hide resolved
config.position = {volumeConfig.length.x() / 2, volumeConfig.length.y() / 2,
0.};
config.volumeCfg = {volumeConfig, volumeConfig2};
Expand Down
Loading