acts-project / acts-project/acts

Multiple context references are in flight for track fitters and finder

Offen
#3,383 3 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Stale
Vorherrschende Sprache
C++
Sterne
131
Forks
276
Ø Merge
3 T. 13 Std.
Gemergte PRs (30 T.)
112

Beschreibung

After https://github.com/acts-project/acts/pull/3181 we have multiple context references in flight for track fitting and finding as they are bound in the algorithm options and propagation options.

https://github.com/acts-project/acts/blob/05c0ebde4aa83baea3c716a3c79cf8481b65817b/Core/include/Acts/TrackFitting/KalmanFitter.hpp#L126-L130
(context if part of `KalmanFitterOptions` and `PropagatorPlainOptions`)

This is not necessarily bad as it consumed little space but might be a source of errors as users could provide two different context objects for a single track fit.

A proposed solution is to decouple the options from the context and always provide context references explicitly as a parameters and never bind to them inside options. This also has the benefit of not making the options non-copyable (in case of a reference) or non-movable (in case of a reference wrapper) or to deal with potentially uninitialized pointers to context objects. A series of PRs are open to showcase these proposed changes:
- https://github.com/acts-project/acts/pull/3378
- https://github.com/acts-project/acts/pull/3379
- https://github.com/acts-project/acts/pull/3381
- https://github.com/acts-project/acts/pull/3380
- https://github.com/acts-project/acts/pull/3382

One downside of this solution is that we potentially end up with a bunch of function parameters. This could be mitigated by binding multiple context objects into a tuple-like struct.

Apart from that it becomes very clear which parts of the code require a context and how this is passed down.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.