Add cuda inhomogeneous bfield type #1017
Conversation
ae89132 to
ae8aa4a
Compare
|
@krasznaa @stephenswat Could you guys check if this is desirable way to include inhomogeneous bfield? |
|
Also feel free to overwrite the PR as it is quite urgent to get the inhomogeneous bfield |
There was a problem hiding this comment.
The formalism for providing the typedefs I like as is already. 😇
The exact covfie types to use are absolutely @stephenswat's to tell us. I'll mainly have input on the code structure of the I/O and device libraries.
| template <typename scalar_t> | ||
| using inhom_bfield_backend_t = covfie::backend::affine<covfie::backend::linear< | ||
| covfie::backend::strided<covfie::vector::vector_d<std::size_t, 3>, | ||
| covfie::backend::cuda_device_array< |
There was a problem hiding this comment.
@stephenswat is the one to tell us this, but we should try texture memory right away, right? I assume that's a different type.
io/include/traccc/io/read_bfield.hpp
Outdated
|
|
||
| /// @brief Function that reads the first 4 bytes of a potential bfield file and | ||
| /// checks that it contains data for a covfie field | ||
| inline bool check_covfie_file(const std::string& file_name) { |
There was a problem hiding this comment.
I don't think that inlining this function is really all that useful. 🤔 This is not performance critical code. Just move it's implementation into a read_bfield.cpp file.
Also, let's do fancy C++20. 😉
| inline bool check_covfie_file(const std::string& file_name) { | |
| bool check_covfie_file(std::string_view file_name); |
io/include/traccc/io/read_bfield.hpp
Outdated
| detray::io::file_handle file{file_name, | ||
| std::ios_base::in | std::ios_base::binary}; |
There was a problem hiding this comment.
Well, this now made me aware of detray::io::file_handle. Which itself I have some thoughts about. 🤔
But that code really doesn't help us much here, does it? All it does in this case is to perform an explicit check on whether the file could be opened as much as I can see.
I would much rather just open an std::ifstream here, with a check making sure that the file could be successfully opened. Avoiding the thread-unfriendliness of detray::io::file_handle that we'll need to address in that eventually.
| /// @brief function that reads a covfie field from file | ||
| template <typename bfield_t> | ||
| inline bfield_t read_bfield(const std::string& file_name) { | ||
|
|
||
| if (!check_covfie_file(file_name)) { | ||
| throw std::runtime_error("Not a valid covfie file: " + file_name); | ||
| } | ||
|
|
||
| // Open binary file | ||
| detray::io::file_handle file{file_name, | ||
| std::ios_base::in | std::ios_base::binary}; | ||
|
|
||
| return bfield_t(*file); | ||
| } |
There was a problem hiding this comment.
Not sure if this function gives us a whole lot. 🤔 I could've imagined a function with an API similar to what we do for the detectors.
https://github.com/acts-project/traccc/blob/main/io/include/traccc/io/read_detector.hpp
I.e. one where we provide a finite set of function overloads for the magnetic field types that we support reading from a binary file. Getting rid of the templating of the user facing function.
But admittedly I don't know enough about covfie still. 😦 What's the general code structure going to be like? I assume we will read in the inhomogeneous field into a host object first, and will copy this into a CUDA, SYCL, etc. field afterwards. I.e. this read function will not be expected to construct a CUDA field directly... right?
202c4d5 to
e4822a8
Compare
|
I have to pause here as I am leaving for vacation from Thursday. I will probably make a couple of tests tomorrow but please feel free to take over the PR any time It was very difficult to extend to inhomogeneous magnetic field so we need to refactor at some point for better maintenance. Having a single type for detector (i.e. default_detector) and bfield (switchable between const and inhom type) might be good as we can remove annoying template and specialization. |
| const unsigned int new_link_buffer_capacity = std::max( | ||
| 2 * link_buffer_capacity, links_size + n_max_candidates); | ||
| /* | ||
| TRACCC_INFO("Link buffer (capacity " |
There was a problem hiding this comment.
Oh yeah I also couldn't use logger outside the algorithm class.. any thought?
|
This PR currently adds instantiations for (1) Host KF with inhomogeneous field (2) CUDA CKF with inhomogeneous field
To-do list