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

Proposal: surface and portal navigation (including target surfaces) for Gen3 #3526

Open
1 of 7 tasks
asalzburger opened this issue Aug 21, 2024 · 5 comments
Open
1 of 7 tasks
Assignees
Labels

Comments

@asalzburger
Copy link
Contributor

asalzburger commented Aug 21, 2024

The current Gen1/Gen2 navigators have to deal with different (implicit) navigation streams:

  • surfaces and portals from the geometry
  • surfaces from external sources
  • target surface(s)

The proposal is to make those streams for Gen3 explicit and base them onto the same implementation:

  • a future navigator would be able to hold several (for the beginning fixed number) of navigation streams that all solve their respective navigation using shared logic & implementation: surface candidates are held by a navigation kernel that is updated by the stream until the navigation kernel is exhausted.
  • these streams are processed by the navigator who then picks the closest solution of all streams and performs the associated navigation action
  • the streams will have an explicit hierarchy otherwise, e.g. target stream overwrites geometry stream

The proposal would be the following 2-3 streams (and associated kernels):

  • target stream: this stream holds the target surface(s) - this can be the final target surface, but also what was formally called external surfaces. The kernel of this stream is not really updated, just the tracing of the candidates internally is updated.

  • geometry stream: this stream comes from the geometry and runs on a kernel (~ similar to what is now called the NavigationState) that is updated by the volumes navigation delegates (either during the stepping) and/or at portal passing.

Test, development sequence:

  • Write a NavigationKernel prototype (first w/o dependence on the stepper for multi-surface approach)
  • Write a StreamNavigator prototype having two different streams
  • Write extensive unit tests of standard and edge cases how such a navigator should behave
  • Fill the StreamNavigator with two streams: sensitives and portals/boundaries from Gen1/Gen2 and demonstrate that correct navigation can be achieved
  • Proof of concept

If/when proof of concept is achieved:

  • change API of NavigationDelegates to fill NavigationKernel instead of NavigationState (or morph NavigationKernel into NavigationState)
  • Run test on Gen2 ODD

Watchpoints:

  • target stream should overwrite geometry stream, i.e. if a surface is in the target stream it should be ignored somehow in a the geometry stream: possible solution is to hand exclusion candidates to the delegate when updating the kernel (i.e. could be a kernel member)
@asalzburger asalzburger self-assigned this Aug 21, 2024
@asalzburger
Copy link
Contributor Author

Super-rough NavigationKernel API:

class NavigationKernel {
 public:
  /// This is a candidate type for the tracing kernel
  /// a Surface : always set
  /// a Portal : set if the surface represents a portal
  /// a size_t : the index of the intersection solution
  /// an ActsScalar : the path length to the intersection
  /// a BoundaryTolerance : the boundary tolerance used for the intersection
  using TracedCandidate = std::tuple<const Surface*, const Portal*, std::size_t,
                                     ActsScalar, BoundaryTolerance>;

  /// Current navigation state:
  /// - the surface for the current surface to be set
  /// - the candidate for the next candidate to be set, nullptr if kernel is
  /// exhausted
  using State = std::tuple<const Surface*, const TracedCandidate*>;

  /// @brief Initialize the tracing kernel
  ///
  /// @param gctx The geometry context
  /// @param position the currevnt position
  /// @param direction the current direction
  /// @param stepSize the estimated step size of the stepper
  /// @param intial if this is the initial call
  ///
  State update(const GeometryContext& gctx, const Vector3& position,
               const Vector3& direction, ActsScalar stepSize, bool initial = false);

 private:
  /// The traced candidates belonging to this kernel
  std::vector<TracedCandidate> m_candidates = {};
  /// The candidate index of the next candidate
  std::array<std::index_t> m_candidateRange = {};

  /// The distance at which the surface is considered to be close enough
  /// for a full intersection check
  ActsScalar m_surfaceProximity = std::numeric_limits<ActsScalar>::max();
};

}  // namespace Acts

@paulgessinger
Copy link
Member

@asalzburger

  1. That's a very complex type and should be a struct with field names.
using TracedCandidate = std::tuple<const Surface*, const Portal*, std::size_t,
                                     ActsScalar, BoundaryTolerance>;
  1. State contains a surface pointer, and TracedCandidate, which contains another surfacer pointer.
  2. update returns by value
  3. What is a kernel?

@asalzburger
Copy link
Contributor Author

Notes:

  • check if TracedCandidate can be an ObjectIntersection + BoundaryTolerance
  • variant type for Portal and Surface
  • Stream is a vector of intersections (object intersections, structs)

@andiwand
Copy link
Contributor

Just for reference I tried to decouple and move some logic from the navigator to the propagator here #3449. This also reduces the number of intersections while approaching a surface with multiple steps.

A few learnings from that procedure

  • Separation makes the jobs of the different components clear and what happens when
  • We can lift position and direction to the propagator which also makes clear that we are in a single place at a single time and what will be/was used for intersections
  • Migration can be split into multiple steps by having two propagation loops during a transition phase so we do not have to port all the navigators and unit tests in a single PR

kodiakhq bot pushed a commit that referenced this issue Aug 30, 2024
This PR introduces a `NavigationStream` object and a corresponding helper. 

The `NavigationStream` should build the backbone of the Gen3 navigator, with having a `TargetStream` and a `GeometryStream` to deal with.

For details of the ongoing discussion, please see #3526.


@paulgessinger @andiwand
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 21, 2024
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

3 participants