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 all 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
48 changes: 25 additions & 23 deletions Core/include/Acts/TrackFitting/KalmanFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -590,6 +590,11 @@ 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;

// Try to find the surface in the measurement surfaces
auto sourceLinkIt = inputMeasurements->find(surface->geometryId());
if (sourceLinkIt != inputMeasurements->end()) {
Expand Down Expand Up @@ -646,36 +651,33 @@ 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 (surface->surfaceMaterial() != nullptr) {
// Update state and stepper with material effects
materialInteractor(surface, state, stepper, navigator,
MaterialUpdateStage::FullUpdate);
if (trackStateProxy.typeFlags().test(TrackStateFlag::HoleFlag)) {
// Count the missed surface
result.missedActiveSurfaces.push_back(surface);
}

++result.processedStates;

// Update state and stepper with (possible) material effects
materialInteractor(surface, state, stepper, navigator,
MaterialUpdateStage::FullUpdate);
}
return Result<void>::success();
}
Expand Down
11 changes: 7 additions & 4 deletions Core/include/Acts/TrackFitting/detail/GsfActor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -634,9 +634,11 @@ 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;
bool isHole = true;

auto cmps = stepper.componentIterable(state.stepping);
for (auto cmp : cmps) {
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 All @@ -656,15 +659,15 @@ struct GsfActor {
const auto& trackStateProxy = *trackStateProxyRes;

if (!trackStateProxy.typeFlags().test(TrackStateFlag::HoleFlag)) {
is_hole = false;
isHole = false;
}

tmpStates.tips.push_back(trackStateProxy.index());
tmpStates.weights[tmpStates.tips.back()] = cmp.weight();
}

// These things should only be done once for all components
if (is_hole) {
if (isHole) {
++result.measurementHoles;
}

Expand Down
16 changes: 12 additions & 4 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 @@ -172,16 +172,24 @@ auto kalmanHandleNoMeasurement(
}

// Set the track state flags
const bool surfaceHasMaterial = surface.surfaceMaterial() != nullptr;
const bool surfaceIsSensitive =
surface.associatedDetectorElement() != nullptr;
auto typeFlags = trackStateProxy.typeFlags();
typeFlags.set(TrackStateFlag::ParameterFlag);
if (surface.surfaceMaterial() != nullptr) {

if (surfaceHasMaterial) {
typeFlags.set(TrackStateFlag::MaterialFlag);
}
if (surface.associatedDetectorElement() != nullptr) {

if (surfaceIsSensitive && 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.surfaceMaterial() != nullptr) {
} else if (surfaceIsSensitive) {
ACTS_VERBOSE("Skip hole (no preceding measurements) on surface "
<< surface.geometryId());
} else if (surfaceHasMaterial) {
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: 798a5b2e6d4b6d56e73ad107887f310c9463cc739ca1b69dad4a99d3419943ca
test_refitting[odd]__tracksummary_gsf_refit.root: 16951808df6363d2acb99e385aec35ad723b634403ca0724a552ae9d3a2ae237
test_refitting[generic]__trackstates_gsf_refit.root: b044574ab5dec9781ca2a1d72095f2393270766365a0e165904ff628191c284a
test_refitting[generic]__tracksummary_gsf_refit.root: 6f8bd054c663197a5f5523e54ebc9695759b671eb14791f964073ed3c8a6f27f
test_truth_tracking_kalman[generic-False-0.0]__trackstates_kf.root: 64a7f26c30f5a70e323dc5d43142a7cc81ad7ef04033095a1ae11d27c41333f0
test_truth_tracking_kalman[generic-False-0.0]__tracksummary_kf.root: 6f8bd054c663197a5f5523e54ebc9695759b671eb14791f964073ed3c8a6f27f
test_refitting[generic]__tracksummary_gsf_refit.root: 5b9ae5be9e82b2a1fd27d54fae5b9d8e89574f7a04a68cd491d4e3761c8fb834
test_truth_tracking_kalman[generic-False-0.0]__trackstates_kf.root: 673a2b6a6fe855e8fcc3f0005a8d598ad61fd696177f54b2fa1af36517ac8f44
test_truth_tracking_kalman[generic-False-0.0]__tracksummary_kf.root: 5b9ae5be9e82b2a1fd27d54fae5b9d8e89574f7a04a68cd491d4e3761c8fb834
test_truth_tracking_kalman[generic-False-1000.0]__trackstates_kf.root: e69d1aacacecc3d7d6619605cdd179d9fb0e19734f7c35345284f9a0e01f36d1
test_truth_tracking_kalman[generic-False-1000.0]__tracksummary_kf.root: 055a74d2747d360398cc846cc2791204491528896de78cca66b188e3ff530dc1
test_truth_tracking_kalman[generic-True-0.0]__trackstates_kf.root: 64a7f26c30f5a70e323dc5d43142a7cc81ad7ef04033095a1ae11d27c41333f0
test_truth_tracking_kalman[generic-True-0.0]__tracksummary_kf.root: 6f8bd054c663197a5f5523e54ebc9695759b671eb14791f964073ed3c8a6f27f
test_truth_tracking_kalman[generic-False-1000.0]__tracksummary_kf.root: 8be4bcc0f05c90ffeb4ea7bbb517adee217550d59c2eed89a48ed3b4c5145c54
test_truth_tracking_kalman[generic-True-0.0]__trackstates_kf.root: 673a2b6a6fe855e8fcc3f0005a8d598ad61fd696177f54b2fa1af36517ac8f44
test_truth_tracking_kalman[generic-True-0.0]__tracksummary_kf.root: 5b9ae5be9e82b2a1fd27d54fae5b9d8e89574f7a04a68cd491d4e3761c8fb834
test_truth_tracking_kalman[generic-True-1000.0]__trackstates_kf.root: e69d1aacacecc3d7d6619605cdd179d9fb0e19734f7c35345284f9a0e01f36d1
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: 22c9f39a8e9569499205c024f7075d4125aca111c0e580c0320d6d93696062d5
test_truth_tracking_kalman[odd-False-0.0]__tracksummary_kf.root: a6da8b8ca2cd0f09fcbae5d08885fa2aee39990f2f329ef659ef3c260141643a
test_truth_tracking_kalman[odd-False-1000.0]__trackstates_kf.root: 214f3ded235a0da8d254e34f72253ee8d6dfa5b7f96040bccd760b51ee85a46f
Expand Down
Loading