-
Notifications
You must be signed in to change notification settings - Fork 168
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
refactor!: std::unique_ptr to std::optional in Acts::PropagatorResult #1622
refactor!: std::unique_ptr to std::optional in Acts::PropagatorResult #1622
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the only advantage of std::unique_ptr
is that the struct is more densely packed. but that doesn't look like a critical property here
Codecov Report
@@ Coverage Diff @@
## main #1622 +/- ##
==========================================
- Coverage 48.65% 48.60% -0.05%
==========================================
Files 381 381
Lines 20780 20826 +46
Branches 9518 9553 +35
==========================================
+ Hits 10110 10122 +12
- Misses 4099 4101 +2
- Partials 6571 6603 +32
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
@andiwand After fixing the unittests etc, could you reapprove |
📊 Physics performance monitoring for dfdd16dFull report VertexingCKFAmbiguity resolutionTruth tracking |
This changes the type that stores the parameters and covariance matrix in the
Acts::PropagatorResult
.I'm not sure why we have
std::unique_ptr
here, maybe this is from pre-C++17 wherestd::optional
was not available? However, both from usibility (not copy-able) and from performance perspective (heap allocation) this does not make sense to me, but maybe I forget something...