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

axpby introduced deep_copies when alpha,beta are scalars #2080

Closed
brian-kelley opened this issue Dec 21, 2023 · 2 comments
Closed

axpby introduced deep_copies when alpha,beta are scalars #2080

brian-kelley opened this issue Dec 21, 2023 · 2 comments

Comments

@brian-kelley
Copy link
Contributor

@eeprude

Tpetra in Trilinos has tests that count the number of Kokkos::deep_copy calls (generally, they want to minimize that number). These tests are failing with KokkosKernels develop branch due to new deep_copies in axpby, probably introduced in #1895 or #2039 . I think it's one of those because they weren't in the latest release, 4.2, which doesn't fail the Tpetra test.

The specific case in Tpetra is where x,y are rank 2, and alpha,beta are scalars. These scalar values are being deep_copied to device views called "managed_a" and "managed_b". But this case should involve no deep_copies, and instead the axpby functor should just have the alpha/beta as members.

The only time there should be a deep_copy for alpha and/or beta is when the user gives them as rank-1 views on host.

It's possible to check that no deep copies are happening by running this case of axpby inside a Kokkos profiling region, and running the kernel logger from Kokkos tools. Here's a good example of this: https://github.com/kokkos/kokkos-tools/wiki/KernelLogger

@eeprude
Copy link
Contributor

eeprude commented Dec 21, 2023

@brian-kelley Thank you for all the hints. I have taken a look at the code and I will work on it during the winter break.

@brian-kelley
Copy link
Contributor Author

@eeprude No, you don't have to do it during break! This isn't even a configuration that people use, it's just for catching issues before we release to Trilinos. It just needs to be happen before we ship 4.3

lucbv added a commit that referenced this issue Jan 8, 2024
Axpby using less deep copy (solves issue #2080)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants