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

LineSurface jacobians seem incomplete #2809

Closed
andiwand opened this issue Dec 11, 2023 · 5 comments
Closed

LineSurface jacobians seem incomplete #2809

andiwand opened this issue Dec 11, 2023 · 5 comments

Comments

@andiwand
Copy link
Contributor

andiwand commented Dec 11, 2023

In the LineSurface we define a boundToFreeJacobian but not a freeToBoundJacobian which seems incorrect to me. If the jacobian is different compared to the one from a plane surface in one direction it should also be the case in the other direction

/// Calculate the jacobian from local to global which the surface knows best,
/// hence the calculation is done here.
///
/// @param gctx The current geometry context object, e.g. alignment
/// @param boundParams is the bound parameters vector
///
/// @return Jacobian from local to global
BoundToFreeMatrix boundToFreeJacobian(
const GeometryContext& gctx, const BoundVector& boundParams) const final;

Acts::BoundToFreeMatrix Acts::LineSurface::boundToFreeJacobian(
const GeometryContext& gctx, const BoundVector& boundParams) const {
// Transform from bound to free parameters
FreeVector freeParams =
detail::transformBoundToFreeParameters(*this, gctx, boundParams);
// The global position
Vector3 position = freeParams.segment<3>(eFreePos0);
// The direction
Vector3 direction = freeParams.segment<3>(eFreeDir0);
// Get the sines and cosines directly
double cosTheta = std::cos(boundParams[eBoundTheta]);
double sinTheta = std::sin(boundParams[eBoundTheta]);
double cosPhi = std::cos(boundParams[eBoundPhi]);
double sinPhi = std::sin(boundParams[eBoundPhi]);
// retrieve the reference frame
auto rframe = referenceFrame(gctx, position, direction);
// Initialize the jacobian from local to global
BoundToFreeMatrix jacToGlobal = BoundToFreeMatrix::Zero();
// the local error components - given by the reference frame
jacToGlobal.topLeftCorner<3, 2>() = rframe.topLeftCorner<3, 2>();
// the time component
jacToGlobal(eFreeTime, eBoundTime) = 1;
// the momentum components
jacToGlobal(eFreeDir0, eBoundPhi) = -sinTheta * sinPhi;
jacToGlobal(eFreeDir0, eBoundTheta) = cosTheta * cosPhi;
jacToGlobal(eFreeDir1, eBoundPhi) = sinTheta * cosPhi;
jacToGlobal(eFreeDir1, eBoundTheta) = cosTheta * sinPhi;
jacToGlobal(eFreeDir2, eBoundTheta) = -sinTheta;
jacToGlobal(eFreeQOverP, eBoundQOverP) = 1;
// the projection of direction onto ref frame normal
double ipdn = 1. / direction.dot(rframe.col(2));
// build the cross product of d(D)/d(eBoundPhi) components with y axis
Vector3 dDPhiY = rframe.block<3, 1>(0, 1).cross(
jacToGlobal.block<3, 1>(eFreeDir0, eBoundPhi));
// and the same for the d(D)/d(eTheta) components
Vector3 dDThetaY = rframe.block<3, 1>(0, 1).cross(
jacToGlobal.block<3, 1>(eFreeDir0, eBoundTheta));
// and correct for the x axis components
dDPhiY -= rframe.block<3, 1>(0, 0) * (rframe.block<3, 1>(0, 0).dot(dDPhiY));
dDThetaY -=
rframe.block<3, 1>(0, 0) * (rframe.block<3, 1>(0, 0).dot(dDThetaY));
// set the jacobian components for global d/ phi/Theta
jacToGlobal.block<3, 1>(eFreePos0, eBoundPhi) =
dDPhiY * boundParams[eBoundLoc0] * ipdn;
jacToGlobal.block<3, 1>(eFreePos0, eBoundTheta) =
dDThetaY * boundParams[eBoundLoc0] * ipdn;
return jacToGlobal;
}

@paulgessinger
Copy link
Member

Maybe @beomki-yeo has thoughts on this.

@beomki-yeo
Copy link
Contributor

beomki-yeo commented Dec 11, 2023

Indeed. I will update my PR on line surface jacobian soon so you can check the details there (#2657)

Summary:
It should not be defined and it's OK not to be defined. J_{G->L} X J_{L->G} is always identical matrix in the current implementation

@andiwand
Copy link
Contributor Author

thanks I will check. still seems odd to me as there should be a correlation between direction error and position error for both transformations

@beomki-yeo
Copy link
Contributor

That's a funny behavior of two different transforms - it will be described in the Discussion section. 😉

@andiwand
Copy link
Contributor Author

thanks for your derivation @beomki-yeo ! I think it makes some sense now to me. I might still need to check this manually to be 100% convinced but it is enough to close this ticket 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants