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

Surface::boundToFreeJacobian could also use FreeVector parameters #2810

Open
andiwand opened this issue Dec 11, 2023 · 1 comment
Open

Surface::boundToFreeJacobian could also use FreeVector parameters #2810

andiwand opened this issue Dec 11, 2023 · 1 comment
Labels

Comments

@andiwand
Copy link
Contributor

It seems inconsistent to use BoundVector for Surface::boundToFreeJacobian and FreeVector for Surface::freeToBoundJacobian. One is basically the inverse of the other and could use the same parameters.

/// Calculate the jacobian from local to global which the surface knows best,
/// hence the calculation is done here.
///
/// @note In principle, the input could also be a free parameters
/// vector as it could be transformed to a bound parameters. But the transform
/// might fail in case the parameters is not on surface. To avoid the check
/// inside this function, it takes directly the bound parameters as input
/// (then the check might be done where this function is called).
///
/// @todo this mixes track parameterisation and geometry
/// should move to :
/// "Acts/EventData/detail/coordinate_transformations.hpp"
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param boundParams is the bound parameters vector
///
/// @return Jacobian from local to global
virtual BoundToFreeMatrix boundToFreeJacobian(
const GeometryContext& gctx, const BoundVector& boundParams) const;

/// Calculate the jacobian from global to local which the surface knows best,
/// hence the calculation is done here.
///
/// @note It assumes the input free parameters is on surface, hence no
/// onSurface check is done inside this function.
///
/// @todo this mixes track parameterisation and geometry
/// should move to :
/// "Acts/EventData/detail/coordinate_transformations.hpp"
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param parameters is the free parameters
///
/// @return Jacobian from global to local
virtual FreeToBoundMatrix freeToBoundJacobian(
const GeometryContext& gctx, const FreeVector& parameters) const;

Internally we always calculate the free parameters and externally we seem to always have the free parameters at hand.

There is also an extreme example where we do the free to bound twice

// Get the local position
const Vector3 position = freeParameters.segment<3>(eFreePos0);
const Vector3 direction = freeParameters.segment<3>(eFreeDir0);
auto lpResult = surface.globalToLocal(geoContext, position, direction);
if (!lpResult.ok()) {
return lpResult.error();
}
// Transform from free to bound parameters
Result<BoundVector> boundParameters = detail::transformFreeToBoundParameters(
freeParameters, surface, geoContext);
if (!boundParameters.ok()) {
return boundParameters.error();
}
// Reset the jacobian from local to global
boundToFreeJacobian =
surface.boundToFreeJacobian(geoContext, *boundParameters);
return Result<void>::success();
}

Copy link

This issue/PR has been automatically marked as stale because it has not had recent activity. The stale label will be removed if any interaction occurs.

@github-actions github-actions bot added the Stale label Jan 10, 2024
kodiakhq bot pushed a commit that referenced this issue Mar 28, 2024
…d of bound vector (#2811)

This avoid free->bound->free roundtrips and aligns the interface with `freeToBoundJacobian`

related issues
- #2810

blocked by
- #2789
- #2782
- #2781
EleniXoch pushed a commit to EleniXoch/acts that referenced this issue May 6, 2024
…d of bound vector (acts-project#2811)

This avoid free->bound->free roundtrips and aligns the interface with `freeToBoundJacobian`

related issues
- acts-project#2810

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
asalzburger pushed a commit to asalzburger/acts that referenced this issue May 21, 2024
…d of bound vector (acts-project#2811)

This avoid free->bound->free roundtrips and aligns the interface with `freeToBoundJacobian`

related issues
- acts-project#2810

blocked by
- acts-project#2789
- acts-project#2782
- acts-project#2781
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant