Skip to content

feat: Adding beam spot as an additional measurement to refitting algorithm #5342

Draft
delitez wants to merge 9 commits intoacts-project:mainfrom
delitez:beamspotKF
Draft

feat: Adding beam spot as an additional measurement to refitting algorithm #5342
delitez wants to merge 9 commits intoacts-project:mainfrom
delitez:beamspotKF

Conversation

@delitez
Copy link
Copy Markdown
Contributor

@delitez delitez commented Apr 13, 2026

In this PR, an option to include beam-spot measurements in the KF refitting algorithm is introduced. If enabled, a measurement at (0,0,0) with beam-spot covariance will be added to source links, using the surface of the current track state.

@github-actions github-actions bot added Component - Examples Affects the Examples module Track Fitting labels Apr 13, 2026
@github-actions github-actions bot added this to the next milestone Apr 13, 2026
@delitez
Copy link
Copy Markdown
Contributor Author

delitez commented Apr 14, 2026

@andiwand could you take a look at this draft PR?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this should be reverted

Comment on lines +412 to +414
if (state.referenceSurface().geometryId().value() == 0u) {
continue;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main limitation is that the source link will not contain an IndexSourceLink. we could generalize this by extending the

if (!state.hasUncalibratedSourceLink()) {

with something like

if (!state.hasUncalibratedSourceLink() || state.getUncalibratedSourceLink().template getPtr<IndexSourceLink>() == nullptr) {

we could also turn the if upside down to first handle the valid source link case and else adding the nans but that is not so important

Comment on lines +111 to +113
if (state.referenceSurface().geometryId().value() == 0u) {
continue;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (state.referenceSurface().geometryId().value() == 0u) {
continue;
}
if (state.getUncalibratedSourceLink().template getPtr<IndexSourceLink>() == nullptr) {
continue;
}

similar like below I think this is more generic

Comment on lines -114 to +167
ACTS_VERBOSE("Initial parameters: "
<< initialParams.fourPosition(ctx.geoContext).transpose()
<< " -> " << initialParams.direction().transpose());
ACTS_VERBOSE("Initial parameters: " << track.parameters().transpose());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the original version seems to have more information

ctx.geoContext,
ctx.magFieldContext,
ctx.calibContext,
&track.referenceSurface(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good to create a new central perigee surface in the refitting algorithm somewhere at the top of execute and supply it also as a reference here

similar to here

// Construct a perigee surface as the target surface
auto pSurface = Acts::Surface::makeShared<Acts::PerigeeSurface>(
Acts::Vector3{0., 0., 0.});

Comment on lines +64 to +72
const Acts::Vector2 beamSpotMeasValue{0., 0.};
Acts::SquareMatrix2 inflatedCov =
Acts::SquareMatrix2::Zero(); //* 12.5 * Acts::UnitConstants::um;
inflatedCov(0, 0) =
12.5 *
Acts::UnitConstants::um;
inflatedCov(1, 1) =
55.5 *
Acts::UnitConstants::mm;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these should be configurable

Comment on lines +37 to +38
/// Add a beam spot measurement
bool addBeamSpotMeasurement = false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we also need the beam spot position and error we could go with an optional like

Suggested change
/// Add a beam spot measurement
bool addBeamSpotMeasurement = false;
/// Add a beam spot measurement
std::optional<Acts::SquareMatrix2> beamSpotConstraint;

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component - Examples Affects the Examples module Track Fitting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants