(CUDA) Templating Rewrite, main branch (2025.06.20.) #1029
Conversation
So that it would follow the same UI that all the other CKF algorithms provide. Had to modify the kernel specialization code a little, as it was itself relying on definitions from the old algorithm class. In sort of a circular way.
So that it would follow the same UI that all the other KF algorithms provide.
780e44c to
314edcc
Compare
| "src/finding/combinatorial_kalman_filter_algorithm_constant_field_default_detector.cu" | ||
| "src/finding/combinatorial_kalman_filter_algorithm_constant_field_telescope_detector.cu" |
There was a problem hiding this comment.
This should go into some kind of specialization directory.
There was a problem hiding this comment.
I guess it could. Though we're not doing that with any of the other libraries either. And it's not really "specialization" that we do here. But rather we split the implementation of the traccc::cuda::combinatorial_kalman_filter_algorithm into multiple source files. As combinatorial_kalman_filter_algorithm.cpp just implements the constructor, while the other two implement the two current "execute functions" of the class.
| "src/fitting/kalman_fitting_algorithm_constant_field_default_detector.cu" | ||
| "src/fitting/kalman_fitting_algorithm_constant_field_telescope_detector.cu" |
| detray::actor_chain<detray::pathlimit_aborter<scalar>, | ||
| detray::parameter_transporter<default_algebra>, | ||
| interaction_register<interactor_t>, interactor_t, | ||
| detray::momentum_aborter<scalar>, ckf_aborter>>; |
There was a problem hiding this comment.
This actor chain can be deduplicated between the different CKF TUs.
There was a problem hiding this comment.
Let me see how to best do this. 🤔
| detray::rk_stepper<bfield_type::view_t, | ||
| default_detector::device::algebra_type, | ||
| detray::constrained_step<scalar_type>>; |
There was a problem hiding this comment.
This RK stepper can also be deduplicated.
| detray::rk_stepper<bfield_type::view_t, | ||
| telescope_detector::device::algebra_type, | ||
| detray::constrained_step<scalar_type>>; |
There was a problem hiding this comment.
Same as above, some refactoring will make this a lot more maintainable.
| #include "traccc/utils/detector_type_utils.hpp" | ||
|
|
||
| namespace traccc::cuda { | ||
| using fitter = fitter_for_t<traccc::telescope_detector::device>; |
There was a problem hiding this comment.
While you're at it, could you specialise these on the B-field?
| #include "traccc/utils/detector_type_utils.hpp" | ||
|
|
||
| namespace traccc::cuda { | ||
| using fitter = fitter_for_t<traccc::telescope_detector::device>; |
There was a problem hiding this comment.
This will also need to specialized on the B-field.
| "src/finding/combinatorial_kalman_filter_algorithm_constant_field_default_detector.cu" | ||
| "src/finding/combinatorial_kalman_filter_algorithm_constant_field_telescope_detector.cu" |
There was a problem hiding this comment.
I guess it could. Though we're not doing that with any of the other libraries either. And it's not really "specialization" that we do here. But rather we split the implementation of the traccc::cuda::combinatorial_kalman_filter_algorithm into multiple source files. As combinatorial_kalman_filter_algorithm.cpp just implements the constructor, while the other two implement the two current "execute functions" of the class.
| detray::actor_chain<detray::pathlimit_aborter<scalar>, | ||
| detray::parameter_transporter<default_algebra>, | ||
| interaction_register<interactor_t>, interactor_t, | ||
| detray::momentum_aborter<scalar>, ckf_aborter>>; |
There was a problem hiding this comment.
Let me see how to best do this. 🤔
| detray::rk_stepper<bfield_type::view_t, | ||
| default_detector::device::algebra_type, | ||
| detray::constrained_step<scalar_type>>; |
While also re-designing the templating in the functions that implement the CKF algorithms for the different backends.
314edcc to
a336d1f
Compare
|
I think I managed to find a way to make the templating int he CKF functions a bit more user friendly. Making the "main functions" specifically templated on the detector and magnetic field types. And then using some newly introduced typedefs to figure out the correct Detray types based on just the detector and magnetic field types. Now on to do the same for the KF algorithms... |
While also slightly re-designing the templating in the functions that implement the KF algorithms for the different backends.
|
This PR ballooned into a much bigger thing by now than I originally intended. But the good news is that after this, adding non-const magnetic field versions of the finding and fitting functions will take a very finite amount of new lines of code. 🤔 |
|
| traccc::details::kalman_fitter_t< | ||
| default_detector::host, | ||
| covfie::field<traccc::const_bfield_backend_t< | ||
| default_detector::host::scalar_type>>::view_t> | ||
| fitter{det, field, m_config}; |
There was a problem hiding this comment.
Technically... this could also be expressed like:
auto fitter = details::make_kalman_fitter(det, field, m_config);(After writing an appropriate helper function of course.)
But let's leave that to a future improvement.
| const measurement_collection_types::const_view& measurements, | ||
| const bound_track_parameters_collection_types::const_view& seeds) const { | ||
|
|
||
| using scalar_type = default_detector::device::scalar_type; |
There was a problem hiding this comment.
Shoot. Forgot to remove this typedef...
Similar to #1026, in preparation for introducing inhomogeneous magnetic fields as discussed in #1017, this PR attempts to harmonize the setup of the CUDA CFK and KF algorithms with how all the other algorithms are implemented by now.
This is of course a big change, but we do need it relatively urgently.
A couple of things to point out:
fill_sort_keyskernel. This was not an issue previously, where only a single template specialization existed for the KF algorithm.traccc::core, but I decided to leave that alone in this PR.propagate_to_next_surfacethat I modified a bit more deeply. In such a way that (hopefully) introducing non-constant magnetic fields should now be a bit easier in a follow-up PR.The clients (in this repository) were all easy to update, but it's worth pointing out to people like @paradajzblond (I couldn't easily find Fabrice's GitHub handle...) that client code will need some changes after this PR.