-
Notifications
You must be signed in to change notification settings - Fork 39
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
Add upper_bound kernel #1170
Add upper_bound kernel #1170
Conversation
Kokkos::View<float *, Kokkos::HostSpace, | ||
Kokkos::MemoryTraits<Kokkos::Unmanaged>>(v_ref.data(), n)); | ||
Kokkos::View<float *, DeviceType> v("Testing::v", n); | ||
Kokkos::deep_copy(space, v, UnmanagedView<float>(v_ref.data(), n)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we care to copy with a specific exec space if we constructed the view without it?
Can't just use create_mirror_view_and_copy() ?
Co-authored-by: Damien L-G <dalg24+github@gmail.com>
cc99046
to
4d6da22
Compare
Addressed the comments, and rebased on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only have minor comments regarding the labelling. I would prefer these to be fixed prior to squash-merging.
space, v, | ||
Kokkos::View<float *, Kokkos::HostSpace, | ||
Kokkos::MemoryTraits<Kokkos::Unmanaged>>(v_ref.data(), n)); | ||
Kokkos::View<float *, DeviceType> v("Testing::v", 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we were going for "[ArborX]{Test,Example}::foo"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use "Testing::foo"
and "Example::foo"
. We use ArborXTest
namespaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please Testing (does not need to be resolved here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please Test
ing(does not need to be resolved here
Are you asking to change all Testing::
to Test::
in a separate PR? Or to ArborXTest::
?
KokkosExt::upper_bound
kernel and testsArborX_Test_DetailsUtils