Repository navigation
Alpaka inhomogeneous magnetic field - #1202
Conversation
…nt Alpaka flavours
Expand on the number of Kalman fit status codes and track fit outcomes to make a more fine-grained distinction between different error types in the Kalman fitter possible. Takes up the improvements in acts-project#1177. Edit: The wire chamber Kalman fitter test had to be adapted, because more error states are recognized Edit2: Also fixed the condition of is_complete for the backward fit to point one position below the first state
If the navigation in the forward KF missed a surface, advance the track state iterator correctly (for example if the propagation config between CKF and KF is different or we are running truth fitting). This is only needed for navigators that are not "guided" or "direct". It also adds an option to the KF to do the full hole count, which is only possible if the propagation starts at the IP and continues through the whole detector, otherwise the first and last holes are missed and the hole count remains an underestimation. Since this will not help in increasing the fit accuracy, but eat more compute resources it will remain optional.
This commit switched the measurement IO code to optionally get the dimensionality from the detector description rather than from the measurement input data.
This commit reworks the covariance estimation logic to match that used in ACTS. It greatly increases the covariances of tracks, hopefully increasing the tracking efficiency.
The CUDA CI jobs are currently running out of disk space. This commit builds ACTS separately, as a release build, in order to save space.
This PR is a revival of acts-project#652, removing the obsolete Kokkos and Futhark code bases.
Did not bother with the host algorithm, because: - It's not actively used in any current workflow; - It works correctly without this optimization as well.
|
Think this is ready to merge. |
krasznaa
left a comment
krasznaa
left a comment
There was a problem hiding this comment.
The feature addition in general is great of course. Let's just arrange the code in a better way. 😉
|
krasznaa
left a comment
krasznaa
left a comment
There was a problem hiding this comment.
Let's discuss in Friday's parallelization meeting about this a little. I'm not absolutely sure what will be the most elegant setup for all this code. 🤔
| option( TRACCC_BUILD_CUDA "Build the CUDA sources included in traccc" FALSE ) | ||
| option( TRACCC_BUILD_CUDA_UTILS "Build the CUDA util sources included in traccc" FALSE ) | ||
| option( TRACCC_BUILD_HIP "Build the HIP sources included in traccc" FALSE) | ||
| option( TRACCC_BUILD_SYCL "Build the SYCL sources included in traccc" FALSE ) | ||
| option( TRACCC_BUILD_SYCL_UTILS "Build the SYCL util sources included in traccc" FALSE ) |
There was a problem hiding this comment.
We haven't done such a thing in traccc yet, but this appears to be a good use-case. Let's use cmake_dependent_option(...) for setting up these flags.
I have the following in mind:
option( TRACCC_BUILD_CUDA_UTILS "Build the CUDA utility sources included in traccc" FALSE )
cmake_dependent_option( TRACCC_BUILD_CUDA "Build the CUDA sources included in traccc"
FALSE TRACCC_BUILD_CUDA_UTILS FALSE )I think this would produce the wanted behaviour, but you should experiment a little. Of course what we want is:
- In case
TRACCC_BUILD_CUDA_UTILS==FALSE, thenTRACCC_BUILD_CUDAmust beFALSE; - if
TRACCC_BUILD_CUDA_UTILS==TRUE, thenTRACCC_BUILD_CUDAmay beTRUE(but is not by default);
And then of course, to make things really fancy, you should make TRACCC_BUILD_ALPAKA depend on the utility flags as well. 🤔
| traccc_add_library( traccc_sycl_utils sycl_utils TYPE SHARED | ||
| # Utilities. | ||
| "include/traccc/sycl/utils/algorithm_base.hpp" | ||
| "src/utils/algorithm_base.sycl" | ||
| "include/traccc/sycl/utils/make_prefix_sum_buff.hpp" | ||
| "src/utils/make_prefix_sum_buff.sycl" | ||
| "include/traccc/sycl/utils/queue_wrapper.hpp" | ||
| "src/utils/queue_wrapper.cpp" | ||
| "include/traccc/sycl/utils/make_magnetic_field.hpp" | ||
| "src/utils/make_magnetic_field.sycl" | ||
| "src/utils/magnetic_field_types.hpp" | ||
| "src/utils/barrier.hpp" | ||
| "src/utils/calculate1DimNdRange.hpp" | ||
| "src/utils/calculate1DimNdRange.sycl" | ||
| "src/utils/get_queue.hpp" | ||
| "src/utils/get_queue.sycl" | ||
| "src/utils/thread_id.hpp" | ||
| "src/utils/oneDPL.hpp" | ||
| "src/sanity/contiguous_on.hpp" | ||
| "src/sanity/ordered_on.hpp" | ||
| ) |
There was a problem hiding this comment.
In both the CUDA and the SYCL case the utility library needs to only hold the code that has a public interface. There is absolutely no need to include stuff like get_queue and calculate1DimNdRange. The utility library should actually be quite little for both CUDA and SYCL.
It might actually turn out that, at least at the moment, you wouldn't even need to link traccc::cuda against traccc::cuda_utils. We may only ever put "utilities" into these libraries that are not direct dependencies of the device algorithms/tools.
Of course if that turns out to be the case, the CMake cache option inter-dependencies simplify a little as well. 🤔
There was a problem hiding this comment.
OK so just magnetic_field at the moment in cuda_utils? I can fix this.
|
I can't join the meeting on Friday but happy to go with whatever's decided (I don't have particularly strong feelings on this). |
|
4 participants
Implementation of alpaka on-device magnetic field. This either calls the CUDA or SYCL versions, or a HIP implementation written here. In principle this could be factorised out as well.
Seems to give reasonable results, although not identical track parameters.
1)AMD HIP:
2)NVidia CUDA:
3)After some delay, Intel (with FP64 emulation):