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

bug: navigation suggests pathCorrection=inf on cylinderSurface #3267

Open
AJPfleger opened this issue Jun 7, 2024 · 6 comments
Open

bug: navigation suggests pathCorrection=inf on cylinderSurface #3267

AJPfleger opened this issue Jun 7, 2024 · 6 comments
Labels
Bug Something isn't working Component - Core Affects the Core module Stale

Comments

@AJPfleger
Copy link
Contributor

Issue

We might encounter a FPEDIV in the path correction of the cylinder surface. This could happen when we end up parallel to the cylinder.

double Acts::CylinderSurface::pathCorrection(

Stacktrace

15:26:16    Sequencer      INFO      -----------------------------------
15:26:16    Sequencer      INFO      FPE summary for Algorithm: TrackFittingAlgorithm
15:26:16    Sequencer      INFO      -----------------------------------
15:26:16    Sequencer      INFO      - FLTDIV: (3 times) 
 0# Acts::CylinderSurface::pathCorrection(Acts::ContextType const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&) const at /builds/acts/ci-bridge/src/Core/src/Surfaces/CylinderSurface.cpp:174
 1# Acts::Layer::compatibleSurfaces(Acts::ContextType const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Eigen::Matrix<double, 3, 1, 0, 3, 1> const&, Acts::NavigationOptions<Acts::Surface> const&) const at /builds/acts/ci-bridge/src/Core/src/Geometry/Layer.cpp:151
 2# bool Acts::Navigator::resolveSurfaces<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::State<Acts::PropagatorOptions<Acts::ActionList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Actor<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> > >, Acts::AbortList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Aborter<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> >, Acts::PathLimitReached> >, Acts::Experimental::Gx2FitterResult<Acts::VectorMultiTrajectory> >, Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer> >(Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::State<Acts::PropagatorOptions<Acts::ActionList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Actor<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> > >, Acts::AbortList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Aborter<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> >, Acts::PathLimitReached> >, Acts::Experimental::Gx2FitterResult<Acts::VectorMultiTrajectory> >&, Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer> const&) const [clone .isra.0] at /builds/acts/ci-bridge/src/Core/include/Acts/Propagator/Navigator.hpp:1055
 3# void Acts::Navigator::preStep<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::State<Acts::PropagatorOptions<Acts::ActionList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Actor<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> > >, Acts::AbortList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Aborter<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> >, Acts::PathLimitReached> >, Acts::Experimental::Gx2FitterResult<Acts::VectorMultiTrajectory> >, Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer> >(Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::State<Acts::PropagatorOptions<Acts::ActionList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Actor<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> > >, Acts::AbortList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Aborter<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> >, Acts::PathLimitReached> >, Acts::Experimental::Gx2FitterResult<Acts::VectorMultiTrajectory> >&, Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer> const&) const at /builds/acts/ci-bridge/src/Core/include/Acts/Propagator/Navigator.hpp:356
 4# Acts::Result<void, std::error_code> Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::propagate<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::State<Acts::PropagatorOptions<Acts::ActionList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Actor<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> > >, Acts::AbortList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Aborter<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> >, Acts::PathLimitReached> >, Acts::Experimental::Gx2FitterResult<Acts::VectorMultiTrajectory> > >(Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>::State<Acts::PropagatorOptions<Acts::ActionList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Actor<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> > >, Acts::AbortList<Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::Aborter<Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> >, Acts::PathLimitReached> >, Acts::Experimental::Gx2FitterResult<Acts::VectorMultiTrajectory> >&) const at /builds/acts/ci-bridge/src/Core/include/Acts/Propagator/Propagator.ipp:48
 5# std::enable_if<!(false), Acts::Result<Acts::TrackContainer<Acts::VectorTrackContainer, Acts::VectorMultiTrajectory, std::shared_ptr>::TrackProxy, std::error_code> >::type Acts::Experimental::Gx2Fitter<Acts::Propagator<Acts::EigenStepper<Acts::StepperExtensionList<Acts::DefaultExtension>, Acts::detail::VoidAuctioneer>, Acts::Navigator>, Acts::VectorMultiTrajectory>::fit<__gnu_cxx::__normal_iterator<Acts::SourceLink const*, std::vector<Acts::SourceLink, std::allocator<Acts::SourceLink> > >, Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis>, Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis>, Acts::VectorTrackContainer, std::shared_ptr, false>(__gnu_cxx::__normal_iterator<Acts::SourceLink const*, std::vector<Acts::SourceLink, std::allocator<Acts::SourceLink> > >, __gnu_cxx::__normal_iterator<Acts::SourceLink const*, std::vector<Acts::SourceLink, std::allocator<Acts::SourceLink> > >, Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> const&, Acts::Experimental::Gx2FitterOptions<Acts::VectorMultiTrajectory> const&, Acts::TrackContainer<Acts::VectorTrackContainer, Acts::VectorMultiTrajectory, std::shared_ptr>&) const at /builds/acts/ci-bridge/src/Core/include/Acts/TrackFitting/GlobalChiSquareFitter.hpp:789
 6# (anonymous namespace)::GlobalChiSquareFitterFunctionImpl::operator()(std::vector<Acts::SourceLink, std::allocator<Acts::SourceLink> > const&, Acts::GenericBoundTrackParameters<Acts::ParticleHypothesis> const&, ActsExamples::TrackFitterFunction::GeneralFitterOptions const&, ActsExamples::MeasurementCalibratorAdapter const&, Acts::TrackContainer<Acts::VectorTrackContainer, Acts::VectorMultiTrajectory, std::shared_ptr>&) const at /builds/acts/ci-bridge/src/Examples/Algorithms/TrackFitting/src/GlobalChiSquareFitterFunction.cpp:110
 7# ActsExamples::TrackFittingAlgorithm::execute(ActsExamples::AlgorithmContext const&) const at /builds/acts/ci-bridge/src/Examples/Algorithms/TrackFitting/src/TrackFittingAlgorithm.cpp:148

Reproduce

Running truth_tracking_gx2f.py with 2.5 < |eta| < 3.0 can result in the above behaviour. When it happens, the gx2f pushed the updated parameters so far, that the first encountered surface is Pixels::Barrel instead of BeamPipe::Barrel:

oldChi2sum = 153.944
chi2sum = 1184.32
13:26:57    Gx2fFitter     DEBUG     chi2 not converging monotonically
13:26:57    Gx2fFitter     VERBOSE   nUpdate = 4/17
13:26:57    Gx2fFitter     VERBOSE   updated params:
loc0:      -31.48 +- 1            1.000
loc1:       10.62 +- 1            0.000  1.000
phi:       -2.856 +- 0.01745      0.000  0.000  1.000
theta:     0.1068 +- 0.01745      0.000  0.000  0.000  1.000
q/p:       0.1268 +- 0.1          0.000  0.000  0.000  0.000  1.000
time:      -239.7 +- 299.8        0.000  0.000  0.000  0.000  0.000  1.000
on surface undefined of type Acts::PerigeeSurface
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Initialization.
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Current surface set to start surface undefined
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Slow start initialization through search.
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Starting from position (-8.8604, 30.2046, 10.6162) and direction (-0.1023, -0.0300, 0.9943)
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Start volume resolved.
13:26:57    Gx2fPropagat   VERBOSE   Path aborter limit set to 41316.4 (full helix = 82632.8, previous limit = 1.79769e+308)
13:26:57    Gx2fPropagat   VERBOSE   Entering propagation.
13:26:57    Gx2fFitterAc   VERBOSE   Surface undefined detected.
13:26:57    Gx2fFitterAc   INFO      Actor: This case is not implemented yet
13:26:57    Gx2fFitterAc   DEBUG     result.processedMeasurements: 0
inputMeasurements.size()13
13:26:57    Gx2fPropagat   VERBOSE   PathLimit aborter | Target stepSize (path limit) updated to ( +∞,  +∞, 41316.4,  +∞)
13:26:57    Gx2fPropagat   VERBOSE   Starting stepping loop.
13:26:57    Gx2fNavigato   VERBOSE   Pixels::Barrel | Entering navigator::preStep.
13:26:57    Gx2fNavigato   VERBOSE   Pixels::Barrel | Start layer to be resolved.
cosAlpha: 0

However, we can also find sometimes values, that are just close to zero:

oldChi2sum = 1184.32
chi2sum = 4.08138
13:26:57    Gx2fFitter     VERBOSE   nUpdate = 5/17
13:26:57    Gx2fFitter     VERBOSE   updated params:
loc0:      -31.08 +- 1            1.000
loc1:       9.863 +- 1            0.000  1.000
phi:       -2.859 +- 0.01745      0.000  0.000  1.000
theta:     0.1068 +- 0.01745      0.000  0.000  0.000  1.000
q/p:       0.1254 +- 0.1          0.000  0.000  0.000  0.000  1.000
time:      -239.7 +- 299.8        0.000  0.000  0.000  0.000  0.000  1.000
on surface undefined of type Acts::PerigeeSurface
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Initialization.
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Current surface set to start surface undefined
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Slow start initialization through search.
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Starting from position (-8.6749, 29.8417, 9.8631) and direction (-0.1024, -0.0298, 0.9943)
13:26:57    Gx2fNavigato   VERBOSE   No Volume | Start volume resolved.
13:26:57    Gx2fPropagat   VERBOSE   Path aborter limit set to 41791.9 (full helix = 83583.9, previous limit = 1.79769e+308)
13:26:57    Gx2fPropagat   VERBOSE   Entering propagation.
13:26:57    Gx2fFitterAc   VERBOSE   Surface undefined detected.
13:26:57    Gx2fFitterAc   INFO      Actor: This case is not implemented yet
13:26:57    Gx2fFitterAc   DEBUG     result.processedMeasurements: 0
inputMeasurements.size()13
13:26:57    Gx2fPropagat   VERBOSE   PathLimit aborter | Target stepSize (path limit) updated to ( +∞,  +∞, 41791.9,  +∞)
13:26:57    Gx2fPropagat   VERBOSE   Starting stepping loop.
13:26:57    Gx2fNavigato   VERBOSE   Pixels::Barrel | Entering navigator::preStep.
13:26:57    Gx2fNavigato   VERBOSE   Pixels::Barrel | Start layer to be resolved.
cosAlpha: 3.46945e-18

Solution

There have been a few attempts to solve this issue:

In the end, we decided not to treat the symptoms, but rather find the underlying issue.

Since it only occurred for the GX2F, this issue might solve itself during further development. However, I don't see it as the GX2F's responsible to check if the navigation will work with newly suggested parameters.

@andiwand
Copy link
Contributor

andiwand commented Jun 7, 2024

13:26:57    Gx2fFitter     VERBOSE   updated params:
loc0:      -31.48 +- 1            1.000
loc1:       10.62 +- 1            0.000  1.000
phi:       -2.856 +- 0.01745      0.000  0.000  1.000
theta:     0.1068 +- 0.01745      0.000  0.000  0.000  1.000
q/p:       0.1268 +- 0.1          0.000  0.000  0.000  0.000  1.000
time:      -239.7 +- 299.8        0.000  0.000  0.000  0.000  0.000  1.000

This suggests that the fitter is unstable for certain inputs / scenarios. I would argue that the navigation is supposed to break in this scenario.

Note that eta for these parameters is almost 3 which is at the edge of what the ODD can handle usually.

I also quickly checked the input vectors and they are practically orthogonal in XY.

In [8]: p = np.array([-8.8604, 30.2046, 10.6162])
In [9]: p[:2] / np.linalg.norm(p[:2])
Out[9]: array([-0.2814848 ,  0.95956569])
In [10]: d = np.array([-0.1023, -0.0300, 0.9943])
In [11]: d[:2] / np.linalg.norm(d[:2])
Out[11]: array([-0.95958925, -0.28140447])

All in all I would argue that this is not a bug of the navigation but a bug of the fitter as the start parameters for the propagation are unphysical for what it is trying to fit.

kodiakhq bot pushed a commit that referenced this issue Jun 7, 2024
Have *almost* same settings for GX2F and KF makes it easier to understand in the physmon output and check if our results are good.

## Why *almost*?
Had also 3 FPEs which fixed here, but did not choose this approach:
- #3254

This issue only occurred for `2.5 < |eta| < 3.0`. To get better physmon until the problem is fixed (maybe in a later stage of the GX2F-development), the eta-range is reduced. The full eta-range could be used after solving:
- #3267
Matthewharri pushed a commit to Matthewharri/acts that referenced this issue Jun 18, 2024
…ct#3248)

Have *almost* same settings for GX2F and KF makes it easier to understand in the physmon output and check if our results are good.

## Why *almost*?
Had also 3 FPEs which fixed here, but did not choose this approach:
- acts-project#3254

This issue only occurred for `2.5 < |eta| < 3.0`. To get better physmon until the problem is fixed (maybe in a later stage of the GX2F-development), the eta-range is reduced. The full eta-range could be used after solving:
- acts-project#3267
Copy link

github-actions bot commented Jul 7, 2024

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.

@AJPfleger
Copy link
Contributor Author

#3411 does not solve the underlying issue but rather avoids getting there in the first place. I would keep it open for now.

@github-actions github-actions bot removed the Stale label Jul 18, 2024
kodiakhq bot pushed a commit that referenced this issue Aug 1, 2024
During the Athena integration, I ran again into the issue described in
- #3267

This PR does not fix the navigation itself but avoids the GX2F ending up in a state, where the above issue would occur. I investigated different approaches to avoid ending up in a situation, where an FPE would occur.

### Simulate division
The FPE occurs, when we are calling the path correction. Checking and aborting if the x-y components for position and direction are zero would be sufficient. However, we should also apply the transformation to the position first, which seemed a bit difficult in the actor.

Using this approach too many tracks were filtered out.
I didn't not check, if all of them would fail anyhow, though.

### Constrain parameters (Used)
I had the issue only, when starting from a volume, that was different from the initial volume of the starting parameters. However, there were also cases, where I switched volumes and didn't crash. After looking into all non-crashing cases (analysed 13 tracks, because then the Athena truth tracking crashed), I saw, that all of them would later end with the error `NotEnoughMeasurements. Therefore, we effectively do not lose any tracks but detect their failure earlier. In the physmon, we also see with larger statistics no change to the current behaviour.

## Testing
I built a new detector for testing this behaviour. I tried to reuse the old one by adding extra parameters, but I didn't succeed. Somehow, I had to flip the detector to force the volume change.

I also added a visualisation script to it, that generates an `.obj` of the detector and draws the measurements. This one I used for the development since the beginning but didn't find a proper place to put it in. The new detector might be complex enough, that we maybe want to visualise it during development.

<img width="1210" alt="image" src="https://github.com/user-attachments/assets/dcf5b91f-f98c-4511-8507-0fc7e4a23ace">

## Blocked
- #3463
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 Aug 17, 2024
@AJPfleger AJPfleger removed the Stale label Aug 27, 2024
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 Sep 27, 2024
@AJPfleger AJPfleger removed the Stale label Oct 1, 2024
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 Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Component - Core Affects the Core module Stale
Projects
None yet
Development

No branches or pull requests

2 participants