Skip to content
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

GPU Track Fitting #296

Merged
merged 2 commits into from
Feb 21, 2023
Merged

GPU Track Fitting #296

merged 2 commits into from
Feb 21, 2023

Conversation

beomki-yeo
Copy link
Contributor

@beomki-yeo beomki-yeo commented Dec 9, 2022

The PR is still in WIP (and depends on #264). I didn't even generalize it for both CUDA and SYCL yet.
But I want to get some advises in advance because I am stuck right now.
The main problem was that I was forced to implement the template classes in the source file:
For example, traccc::cuda::fitting_algorithm is declared in traccc/device/cuda/include/traccc/cuda/fitting/fitting_algorithm.hpp and implemented in traccc/device/cuda/src/fitting/fitting_algorithm.cu
Because traccc::cuda::fitting_algorithm is a template class, it was necessary to explicitly instantiate explicitly the class with specific template parameters :/ (See the code below)

// Explicit template instantiation
using host_detector_type =
    detray::detector<detray::detector_registry::telescope_detector,
                     covfie::field>;
using device_detector_type =
    detray::detector<detray::detector_registry::telescope_detector,
                     covfie::field_view, detray::device_container_types>;
using b_field_t = typename host_detector_type::bfield_type;
using rk_stepper_type = detray::rk_stepper<b_field_t::view_t, transform3,
                                           detray::constrained_step<>>;
using device_navigator_type = detray::navigator<const device_detector_type>;
using device_fitter_type =
    kalman_fitter<rk_stepper_type, device_navigator_type>;
template class fitting_algorithm<device_fitter_type, host_detector_type>;

Instantiating the classes for every detector class is definitely not I want to do. I wonder if there is a workaround for this ?

@beomki-yeo
Copy link
Contributor Author

I am also not sure if I can get rid of .cu implementation and put everything including kernels into the header file

@beomki-yeo beomki-yeo marked this pull request as draft December 16, 2022 20:10
@beomki-yeo beomki-yeo mentioned this pull request Jan 6, 2023
@beomki-yeo
Copy link
Contributor Author

I will be back after I make detray work with SYCL

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general I like it. But we'll unfortunately need to find some replacement for the recursive function call(s). At least for SYCL. (But I think CUDA would also benefit from not relying on recursive calls.)

@beomki-yeo
Copy link
Contributor Author

But we'll unfortunately need to find some replacement for the recursive function call(s). At least for SYCL.

It will be resolved by acts-project/detray#369. There was only one recursive function from std::sort, and I implemented a custom sort function to work around it.

@stephenswat stephenswat added feature New feature or request cuda Changes related to CUDA sycl Changes related to SYCL labels Jan 25, 2023
@beomki-yeo beomki-yeo marked this pull request as ready for review January 30, 2023 14:00
Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With some tremendous delay, let's finally push this in...

Copy link
Member

@krasznaa krasznaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@krasznaa krasznaa merged commit f14dcf5 into acts-project:main Feb 21, 2023
@beomki-yeo beomki-yeo mentioned this pull request Feb 27, 2023
stephenswat added a commit to stephenswat/traccc that referenced this pull request Mar 23, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
stephenswat added a commit to stephenswat/traccc that referenced this pull request Mar 24, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
stephenswat added a commit to stephenswat/traccc that referenced this pull request Mar 24, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
stephenswat added a commit to stephenswat/traccc that referenced this pull request Mar 28, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
stephenswat added a commit to stephenswat/traccc that referenced this pull request Mar 28, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
stephenswat added a commit to stephenswat/traccc that referenced this pull request May 10, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
stephenswat added a commit to stephenswat/traccc that referenced this pull request May 11, 2023
PR acts-project#296 introduces a dependency on some of the covfie targets, but does
not actually attempt to load the library (and thus the targets). This
commit adds functionality to the build system to do so.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda Changes related to CUDA feature New feature or request sycl Changes related to SYCL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants