refactor: Reuse MultiTrajectory in (C)KF #1507
Conversation
|
@benjaminhuth No I haven't but that's actually a good point. |
|
I ran KF, CKF and GSF examples to get a rough idea of the performance:
It seems CKF and KF are slightly faster, while GSF gets substantially slower. This is average + stdev of 10 runs each. @benjaminhuth Any idea why this change might be affecting GSF differently than (C)KF? Overhead from |
|
Ok I think I managed to bring down the GSF example runtime on this branch to something like 6.043 +- 0.018s by lifting where it resolves if an input trajectory is given. That might be acceptable, @benjaminhuth? |
|
Interesting that the position where the MT is constructed has such a big impact on the performance... However, GSF performance is a topic that must be addressed in general at some point, so that is totally fine with the change. |
Codecov Report
@@ Coverage Diff @@
## main #1507 +/- ##
==========================================
- Coverage 48.59% 48.58% -0.01%
==========================================
Files 381 381
Lines 20631 20649 +18
Branches 9463 9475 +12
==========================================
+ Hits 10026 10033 +7
+ Misses 4066 4064 -2
- Partials 6539 6552 +13
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I think this is working now, can you reapprove @benjaminhuth? |
This PR changes the CKF and KF to reuse the same MultiTrajectory. CKF does this internally, retaining a single MTJ outside the for loop over the seeds. KF's `fit()` method now accepts an optional `trajectory` argument, that it will append track states into. If it's not given, it will create a fresh MTJ. This cuts down on the number of objects we have flying around. In the post-processing of CKF outputs via `Trajectory` objects, all of the objects per event will now point to the same MTJ. I plan to refactor this when the Track EDM lands.
This PR changes the CKF and KF to reuse the same MultiTrajectory. CKF does this internally, retaining a single MTJ outside the for loop over the seeds. KF's
fit()method now accepts an optionaltrajectoryargument, that it will append track states into. If it's not given, it will create a fresh MTJ.This cuts down on the number of objects we have flying around.
In the post-processing of CKF outputs via
Trajectoryobjects, all of the objects per event will now point to the same MTJ. I plan to refactor this when the Track EDM lands.See #1516