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: bad_variant_access in SympyStepper #3523

Closed
timadye opened this issue Aug 20, 2024 · 6 comments
Closed

bug: bad_variant_access in SympyStepper #3523

timadye opened this issue Aug 20, 2024 · 6 comments

Comments

@timadye
Copy link
Contributor

timadye commented Aug 20, 2024

full_chain_itk.py crashes in the first event with ttbar_pu200=True:

terminate called after throwing an instance of 'std::bad_variant_access'
  what():  std::get: wrong index for variant

The gdb traceback includes:

#11 0x00007fffe8201d81 in std::__throw_bad_variant_access (__valueless=<optimized out>) at /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/variant:1333
#12 std::get<0ul, Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::error_code> (__v=...) at /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/variant:1692
#13 std::get<Eigen::Matrix<double, 3, 1, 0, 3, 1>, Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::error_code> (__v=...) at /cvmfs/sft.cern.ch/lcg/releases/gcc/13.1.0-b3d18/x86_64-el9/include/c++/13.1.0/variant:1128
#14 Acts::Result<Eigen::Matrix<double, 3, 1, 0, 3, 1>, std::error_code>::operator* (this=0x7fffffff3500) at /home/ppd/adye/acts/build/src/Core/include/Acts/Utilities/Result.hpp:118
#15 operator() (__closure=<synthetic pointer>, p=<synthetic pointer>) at /home/ppd/adye/acts/build/src/Core/src/Propagator/SympyStepper.cpp:121
#16 rk4<double, Acts::SympyStepper::stepImpl(State&, Acts::Direction, double, double, std::size_t) const::<lambda(double const*)> > (J=<optimized out>, path_derivatives=<optimized out>, new_time=<optimized out>, new_d=0x7fffffff4ef0,
    new_p=0x7fffffff4ed0, errTol=<optimized out>, err=<synthetic pointer>, getB=..., p_abs=<optimized out>, m=<optimized out>, lambda=0.080233294464808397, h=<optimized out>, t=<optimized out>, d=<synthetic pointer>, p=<synthetic pointer>)
    at /home/ppd/adye/acts/build/src/Core/src/Propagator/codegen/sympy_stepper_math.hpp:38
#17 Acts::SympyStepper::stepImpl (this=this@entry=0x1da955f0, state=..., stepDirection=..., stepTolerance=0.0001, stepSizeCutOff=0, maxRungeKuttaStepTrials=10000) at /home/ppd/adye/acts/build/src/Core/src/Propagator/SympyStepper.cpp:159
#18 0x00007fffe920aa55 in Acts::SympyStepper::step<Acts::PropagatorState<Acts::PropagatorOptions<Acts::SympyStepper::Options, Acts::Navigator::Options, Acts::ActionList<ActsFatras::detail::SimulationActor<std::mersenne_twister_engine<unsigned long, 32, 624, 397, 31, 2567483615, 11, 4294967295, 7, 2636928640, 15, 4022730752, 18, 1812433253>, ActsFatras::NoDecay, ActsFatras::InteractionList<ActsFatras::ContinuousProcess<ActsFatras::GenericScattering<ActsFatras::detail::Highland>, ActsFatras::ChargedSelector, ActsFatras::EveryParticle, ActsFatras::EveryParticle>, ActsFatras::ContinuousProcess<ActsFatras::BetheBloch, ActsFatras::ChargedSelector, ActsFatras::Min<ActsFatras::Casts::P>, ActsFatras::EveryParticle>, ActsFatras::ContinuousProcess<ActsFatras::BetheHeitler, ActsFatras::AbsPdgSelector<(Acts::PdgParticle)11>, ActsFatras::Min<ActsFatras::Casts::P>, ActsFatras::Min<ActsFatras::Casts::P> > >, (anonymous namespace)::HitSurfaceSelector> >, Acts::AbortList<ActsFatras::detail::SimulationActor<std::mersenne_twister_engine<unsigned long, 32, 624, 397, 31, 2567483615, 11, 4294967295, 7, 2636928640, 15, 4022730752, 18, 1812433253>, ActsFatras::NoDecay, ActsFatras::InteractionList<ActsFatras::ContinuousProcess<ActsFatras::GenericScattering<ActsFatras::detail::Highland>, ActsFatras::ChargedSelector, ActsFatras::EveryParticle, ActsFatras::EveryParticle>, ActsFatras::ContinuousProcess<ActsFatras::BetheBloch, ActsFatras::ChargedSelector, ActsFatras::Min<ActsFatras::Casts::P>, ActsFatras::EveryParticle>, ActsFatras::ContinuousProcess<ActsFatras::BetheHeitler, ActsFatras::AbsPdgSelector<(Acts::PdgParticle)11>, ActsFatras::Min<ActsFatras::Casts::P>, ActsFatras::Min<ActsFatras::Casts::P> > >, (anonymous namespace)::HitSurfaceSelector>::ParticleNotAlive, Acts::EndOfWorldReached, Acts::PathLimitReached> >, Acts::SympyStepper::State, Acts::Navigator::State, ActsFatras::SimulationResult>, Acts::Navigator> (state=..., this=0x1da955f0)
    at /home/ppd/adye/acts/build/src/Core/include/Acts/Propagator/SympyStepper.ipp:14

I am using acts main (320ef1beb, 2024-08-20) built with LCG_105a/x86_64-el9-gcc13-opt. I think this is a critical bug.

@andiwand
Copy link
Contributor

looking at the code this seems to be the usual "outside magnetic field" error which is not communicated correctly in the SympyStepper

return *fieldRes;

I remember not wiring this because it interacts with the generated code and error handling is hard there.

I see two solutions

  1. implement proper error handling in the generated code. basically this should just be an if (bad) return error
  2. throw an exception and catch it inside the stepper. basically side stepping the current control flow

@timadye
Copy link
Contributor Author

timadye commented Aug 20, 2024

Would a third option be:
3. return zero magnetic field (0,0,0) or some other special value, and let the caller resolve it

@andiwand
Copy link
Contributor

That would also be a tempting solution in this case. The error was kind of helpful in the past because it was the only flag raised when we left the detector. But at the same time we buy quite some branching and control flow complexity in very hot code paths. What do you think @paulgessinger ?

@andiwand
Copy link
Contributor

Generally this is one of the problems with an underlying cause. We can return B=(0,0,0) and it will fix the segfault in this case but there will still be propagations leaving the tracking geometry without but then without notice.

I think the underlying problem here is that we generate particles with pythia8 at the perigee as reference surface but some of them are quite displaced and end up in the middle of the tracking geometry. I had similar issues here #3442. If we cut them inside the beampipe the propagation problem might disappear.

kodiakhq bot pushed a commit that referenced this issue Aug 20, 2024
A missing B field is not correctly handled in the `SympyStepper` and can result in segmentation faults.

Fixes
- #3523
@asalzburger
Copy link
Contributor

This can be closed, as the fix was merged - first @timadye - could you confirm that the issue is fixed with #3525 and close it accordingly?

@timadye
Copy link
Contributor Author

timadye commented Aug 21, 2024

Yes, I can confirm this fixes the issue for me (main 7c27231).

For your information, since I have run this test, I compared the performance of 100 ttbar_pu200 events with full_chain_itk.py (8 threads) with the latest main and the last time I ran this test on main (30/07/2024, 68c92a6). I see a 23% speedup of the TrackFindingAlgorithm, though some changes in physics performance:
trackeff_vs_eta
nOutliers_vs_eta
fakerate_vs_eta
The speedup is very welcome and the performance changes are probably fine, though the fake rates are are a little concerning.
The changes surely have nothing to do with this fix but is one of the other recent changes, so I close this issue.

@timadye timadye closed this as completed Aug 21, 2024
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