Skip to content

Commit

Permalink
fix: kf+gsf: correct hole-tagging for edge case (#3637)
Browse files Browse the repository at this point in the history 
We want to add the `HoleFlag`, when `(precedingMeasurementExists && surfaceIsSensitive)`. We had the case, that we would wrongly always tag holes, when `surfaceIsSensitive && surfaceHasMaterial` because then the check for `precedingMeasurementExists` would have been short-circuited.

The physmon does no change, because there we use the ODD which has material and sensitive on different surface, therefore could not be short-circuited. That's why we only see a difference in the generic detector hashes.

blocked:
- #3647
  • Loading branch information
@AJPfleger
AJPfleger authored Sep 26, 2024
1 parent 008f1b3 commit 495c60d
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 38 deletions.
48 changes: 25 additions & 23 deletions Core/include/Acts/TrackFitting/KalmanFitter.hpp
View file
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);

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
View file
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
View file
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
View file
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

0 comments on commit 495c60d

Please sign in to comment.