Skip to content

Commit

Permalink
fix(gx2f): new error UsedUnreachableMeasurements (#3653)
Browse files Browse the repository at this point in the history
This fixes a super rare bug. 1 in 17k segfaulted at `calculateTrackQuantities(track)`. This was, because the `tipIndex` was set outside the actual track. The wrong `tipIndex` comes from the last propagation where we didn't try to hit all `inputMeasurements` on `insertExternalSurface`s.

The created track contains measurement information, that is missing in the final track. We could think about having this as an outlier or refit ignoring the missed measurements. For now, I would just return an error, because:
- We have no proper outlier definition in the GX2F yet
- Refitting should be maybe a user-task. The whole refitting logic with a dropped measurement might make the whole fitter quite complicated
- The error is so rare, that it is fine, to just avoid crashing

We discussed an would refrain from creating a unit test, since it is quite difficult to reproduce this edge case in an isolated environment. Due to its rarity, it might be not worth the time.
  • Loading branch information
AJPfleger authored Oct 2, 2024
1 parent 11be339 commit 661ab12
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
37 changes: 33 additions & 4 deletions Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -610,10 +610,10 @@ class Gx2Fitter {

// Check if we can stop to propagate
if (result.measurementStates == inputMeasurements->size()) {
ACTS_INFO("Actor: finish: All measurements have been found.");
ACTS_DEBUG("Actor: finish: All measurements have been found.");
result.finished = true;
} else if (state.navigation.navigationBreak) {
ACTS_INFO("Actor: finish: navigationBreak.");
ACTS_DEBUG("Actor: finish: navigationBreak.");
result.finished = true;
}

Expand Down Expand Up @@ -1218,7 +1218,7 @@ class Gx2Fitter {

// We only consider states with a measurement (and/or material)
if (!stateHasMeasurement && !doMaterial) {
ACTS_INFO(" Skip state.");
ACTS_DEBUG(" Skip state.");
continue;
}

Expand Down Expand Up @@ -1388,6 +1388,9 @@ class Gx2Fitter {

// Propagate again with the final covariance matrix. This is necessary to
// obtain the propagated covariance for each state.
// We also need to recheck the result and find the tipIndex, because at this
// step, we will not ignore the boundary checks for measurement surfaces. We
// want to create trackstates only on surfaces, that we actually hit.
if (gx2fOptions.nUpdateMax > 0) {
ACTS_VERBOSE("final deltaParams:\n" << deltaParams);
ACTS_VERBOSE("Propagate with the final covariance.");
Expand All @@ -1413,7 +1416,33 @@ class Gx2Fitter {
auto& r = propagatorState.template get<Gx2FitterResult<traj_t>>();
r.fittedStates = &trackContainer.trackStateContainer();

m_propagator.template propagate(propagatorState);
auto propagationResult = m_propagator.template propagate(propagatorState);

// Run the fitter
auto result = m_propagator.template makeResult(std::move(propagatorState),
propagationResult,
propagatorOptions, false);

if (!result.ok()) {
ACTS_ERROR("Propagation failed: " << result.error());
return result.error();
}

auto& propRes = *result;
GX2FResult gx2fResult = std::move(propRes.template get<GX2FResult>());

if (!gx2fResult.result.ok()) {
ACTS_INFO("GlobalChiSquareFitter failed in actor: "
<< gx2fResult.result.error() << ", "
<< gx2fResult.result.error().message());
return gx2fResult.result.error();
}

if (tipIndex != gx2fResult.lastMeasurementIndex) {
ACTS_INFO("Final fit used unreachable measurements.");
return Experimental::GlobalChiSquareFitterError::
UsedUnreachableMeasurements;
}
}

if (!trackContainer.hasColumn(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ enum class GlobalChiSquareFitterError {
DidNotConverge = 2,
NotEnoughMeasurements = 3,
UpdatePushedToNewVolume = 4,
UsedUnreachableMeasurements = 5,
};

std::error_code make_error_code(
Expand Down
2 changes: 2 additions & 0 deletions Core/src/TrackFitting/GlobalChiSquareFitterError.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ class GlobalChiSquareFitterErrorCategory : public std::error_category {
return "Gx2f: Not enough measurements.";
case GlobalChiSquareFitterError::UpdatePushedToNewVolume:
return "Gx2f: Update pushed the parameters to a new volume.";
case GlobalChiSquareFitterError::UsedUnreachableMeasurements:
return "Gx2f: Final fit used unreachable measurements.";
default:
return "unknown";
}
Expand Down

0 comments on commit 661ab12

Please sign in to comment.