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: kf+gsf: correct hole-tagging for edge case #3637

Merged
merged 20 commits into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
44 changes: 24 additions & 20 deletions Core/include/Acts/TrackFitting/KalmanFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,8 +582,13 @@ class KalmanFitter {
Result<void> filter(const Surface* surface, propagator_state_t& state,
const stepper_t& stepper, const navigator_t& navigator,
result_type& result) const {
const bool precedingMeasurementExists = (result.measurementStates > 0);
const bool surfaceIsSensitive =
(surface->associatedDetectorElement() != nullptr);
const bool surfaceHasMaterial = (surface->surfaceMaterial() != nullptr);
AJPfleger marked this conversation as resolved.
Show resolved Hide resolved
const auto sourcelink_it = inputMeasurements->find(surface->geometryId());
AJPfleger marked this conversation as resolved.
Show resolved Hide resolved

// Try to find the surface in the measurement surfaces
auto sourcelink_it = inputMeasurements->find(surface->geometryId());
if (sourcelink_it != inputMeasurements->end()) {
// Screen output message
ACTS_VERBOSE("Measurement surface " << surface->geometryId()
Expand Down Expand Up @@ -638,32 +643,31 @@ class KalmanFitter {
// the lastTrackIndex.
result.lastMeasurementIndex = result.lastTrackIndex;

} else if (surface->associatedDetectorElement() != nullptr ||
surface->surfaceMaterial() != nullptr) {
} else if ((precedingMeasurementExists && surfaceIsSensitive) ||
surfaceHasMaterial) {
// We only create track states here if there is already measurement
// detected or if the surface has material (no holes before the first
// measurement)
if (result.measurementStates > 0 ||
surface->surfaceMaterial() != nullptr) {
auto trackStateProxyRes = detail::kalmanHandleNoMeasurement(
state, stepper, *surface, *result.fittedStates,
result.lastTrackIndex, true, logger(), freeToBoundCorrection);

if (!trackStateProxyRes.ok()) {
return trackStateProxyRes.error();
}
auto trackStateProxyRes = detail::kalmanHandleNoMeasurement(
state, stepper, *surface, *result.fittedStates,
result.lastTrackIndex, true, logger(), precedingMeasurementExists,
freeToBoundCorrection);
AJPfleger marked this conversation as resolved.
Show resolved Hide resolved

const auto& trackStateProxy = *trackStateProxyRes;
result.lastTrackIndex = trackStateProxy.index();
if (!trackStateProxyRes.ok()) {
return trackStateProxyRes.error();
}

if (trackStateProxy.typeFlags().test(TrackStateFlag::HoleFlag)) {
// Count the missed surface
result.missedActiveSurfaces.push_back(surface);
}
const auto& trackStateProxy = *trackStateProxyRes;
result.lastTrackIndex = trackStateProxy.index();

++result.processedStates;
if (trackStateProxy.typeFlags().test(TrackStateFlag::HoleFlag)) {
// Count the missed surface
result.missedActiveSurfaces.push_back(surface);
}
if (surface->surfaceMaterial() != nullptr) {

++result.processedStates;

if (surfaceHasMaterial) {
AJPfleger marked this conversation as resolved.
Show resolved Hide resolved
// Update state and stepper with material effects
materialInteractor(surface, state, stepper, navigator,
MaterialUpdateStage::FullUpdate);
Expand Down
5 changes: 4 additions & 1 deletion Core/include/Acts/TrackFitting/detail/GsfActor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,8 @@ struct GsfActor {
bool doCovTransport) const {
const auto& surface = *navigator.currentSurface(state.navigation);

const bool precedingMeasurementExists = result.processedStates > 0;

// Initialize as true, so that any component can flip it. However, all
// components should behave the same
bool is_hole = true;
Expand All @@ -647,7 +649,8 @@ struct GsfActor {
// now until we measure this is significant
auto trackStateProxyRes = detail::kalmanHandleNoMeasurement(
singleState, singleStepper, surface, tmpStates.traj,
MultiTrajectoryTraits::kInvalid, doCovTransport, logger());
MultiTrajectoryTraits::kInvalid, doCovTransport, logger(),
precedingMeasurementExists);

if (!trackStateProxyRes.ok()) {
return trackStateProxyRes.error();
Expand Down
8 changes: 6 additions & 2 deletions Core/include/Acts/TrackFitting/detail/KalmanUpdateHelpers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ template <typename propagator_state_t, typename stepper_t, typename traj_t>
auto kalmanHandleNoMeasurement(
propagator_state_t &state, const stepper_t &stepper, const Surface &surface,
traj_t &fittedStates, const std::size_t lastTrackIndex, bool doCovTransport,
const Logger &logger,
const Logger &logger, const bool precedingMeasurementExists,
const FreeToBoundCorrection &freeToBoundCorrection = FreeToBoundCorrection(
false)) -> Result<typename traj_t::TrackStateProxy> {
// Add a <mask> TrackState entry multi trajectory. This allocates storage for
Expand Down Expand Up @@ -177,10 +177,14 @@ auto kalmanHandleNoMeasurement(
if (surface.surfaceMaterial() != nullptr) {
typeFlags.set(TrackStateFlag::MaterialFlag);
}
if (surface.associatedDetectorElement() != nullptr) {
if (surface.associatedDetectorElement() != nullptr &&
AJPfleger marked this conversation as resolved.
Show resolved Hide resolved
precedingMeasurementExists) {
ACTS_VERBOSE("Detected hole on " << surface.geometryId());
// If the surface is sensitive, set the hole type flag
typeFlags.set(TrackStateFlag::HoleFlag);
} else if (surface.associatedDetectorElement() != nullptr) {
ACTS_VERBOSE("Skip hole (no preceding measurements) on surface "
<< surface.geometryId());
} else if (surface.surfaceMaterial() != nullptr) {
ACTS_VERBOSE("Detected in-sensitive surface " << surface.geometryId());
}
Expand Down
14 changes: 7 additions & 7 deletions Examples/Python/tests/root_file_hashes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ test_ML_Ambiguity_Solver__performance_ambiML.root: 284ff5c3a08c0b810938e4ac2f8ba
test_refitting[odd]__trackstates_gsf_refit.root: 33de1e46b05abc450ff89aebc728d7cb85e3f4dc90e5a21fc79a1ed8e9e04d7d
test_refitting[odd]__tracksummary_gsf_refit.root: 16951808df6363d2acb99e385aec35ad723b634403ca0724a552ae9d3a2ae237
test_refitting[generic]__trackstates_gsf_refit.root: ae07a875975a416d323d98b4b16dd3edc5ae672a8993ecb624758a770afdffbd
test_refitting[generic]__tracksummary_gsf_refit.root: bf46a89e429fa77a380d5ee48babb8af2044196eff872825e84c0d4401357114
test_truth_tracking_kalman[generic-False-0.0]__trackstates_kf.root: c2a351bb12b81a69549ca0229d36369cb63cbc53a924f0fd0520879d214b9134
test_truth_tracking_kalman[generic-False-0.0]__tracksummary_kf.root: bf46a89e429fa77a380d5ee48babb8af2044196eff872825e84c0d4401357114
test_refitting[generic]__tracksummary_gsf_refit.root: 9dd347fcab40d1045bd48e1d7e531fbc9f1e07a2e7e321f196250721befe086c
test_truth_tracking_kalman[generic-False-0.0]__trackstates_kf.root: 4e49258ae95ef7b7c59ef155545323fbee6de3f160a0cd975225c0458ec27912
test_truth_tracking_kalman[generic-False-0.0]__tracksummary_kf.root: 9dd347fcab40d1045bd48e1d7e531fbc9f1e07a2e7e321f196250721befe086c
test_truth_tracking_kalman[generic-False-1000.0]__trackstates_kf.root: 0a2428ff260466f7fe4ea44863664afa3c7e34b0dc388e43ee096d132f3acefb
test_truth_tracking_kalman[generic-False-1000.0]__tracksummary_kf.root: 055a74d2747d360398cc846cc2791204491528896de78cca66b188e3ff530dc1
test_truth_tracking_kalman[generic-True-0.0]__trackstates_kf.root: c2a351bb12b81a69549ca0229d36369cb63cbc53a924f0fd0520879d214b9134
test_truth_tracking_kalman[generic-True-0.0]__tracksummary_kf.root: bf46a89e429fa77a380d5ee48babb8af2044196eff872825e84c0d4401357114
test_truth_tracking_kalman[generic-False-1000.0]__tracksummary_kf.root: 8be4bcc0f05c90ffeb4ea7bbb517adee217550d59c2eed89a48ed3b4c5145c54
test_truth_tracking_kalman[generic-True-0.0]__trackstates_kf.root: 4e49258ae95ef7b7c59ef155545323fbee6de3f160a0cd975225c0458ec27912
test_truth_tracking_kalman[generic-True-0.0]__tracksummary_kf.root: 9dd347fcab40d1045bd48e1d7e531fbc9f1e07a2e7e321f196250721befe086c
test_truth_tracking_kalman[generic-True-1000.0]__trackstates_kf.root: 0a2428ff260466f7fe4ea44863664afa3c7e34b0dc388e43ee096d132f3acefb
test_truth_tracking_kalman[generic-True-1000.0]__tracksummary_kf.root: 055a74d2747d360398cc846cc2791204491528896de78cca66b188e3ff530dc1
test_truth_tracking_kalman[generic-True-1000.0]__tracksummary_kf.root: 8be4bcc0f05c90ffeb4ea7bbb517adee217550d59c2eed89a48ed3b4c5145c54
test_truth_tracking_kalman[odd-False-0.0]__trackstates_kf.root: a34546ba6cfdbecc3bcf6f02ec782ebcc76f332070e7ff8a64207ed815f65c7e
test_truth_tracking_kalman[odd-False-0.0]__tracksummary_kf.root: 7f1bb68b39e52da7a77ea295819fa3776e947568455338738c3a6436c2d7599c
test_truth_tracking_kalman[odd-False-1000.0]__trackstates_kf.root: 0344aa50ba6c14152cf0f29b6f6da8af88fd61556a99baa62bef2fdbbc121ac8
Expand Down
Loading