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

Allow non device type template parameter for output views in query() on distributed trees #349

Merged
merged 6 commits into from
Jul 31, 2020

Conversation

dalg24
Copy link
Contributor

@dalg24 dalg24 commented Jul 30, 2020

Basically #271 but for distributed trees

Cleaning up distributed tree implementation details is more involved than anticipated.
My short term plan is to get all queryDispatch overloads out of the struct templated on the device type and
then to introduce the new overloads that take a view of "pair" local index + rank instead of two separate views.

Copy link
Contributor

@aprokop aprokop left a comment

Choose a reason for hiding this comment

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

Looks reasonable at a glance.

Co-authored-by: Andrey Prokopenko <andrey.prok@gmail.com>
@aprokop aprokop merged commit ffed0fe into arborx:master Jul 31, 2020
typename Ranks>
static std::enable_if_t<Kokkos::is_view<Indices>{} &&
Kokkos::is_view<Offset>{} && Kokkos::is_view<Ranks>{}>
queryDispatch(SpatialPredicateTag, DistributedTree const &tree,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want to allow any other DistributedTree in this pull request?
Otherwise, I would rather use

Suggested change
queryDispatch(SpatialPredicateTag, DistributedTree const &tree,
queryDispatch(SpatialPredicateTag, DistributedSearchTree<MemorySpace, void> const &tree,

(here and below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to get rid of the forward declaration. Also this is one less place to update if we add template parameters to the distributed tree.

ExecutionSpace const &space, Predicates const &queries,
Kokkos::View<int *, DeviceType> &indices,
Kokkos::View<int *, DeviceType> &offset,
Kokkos::View<int *, DeviceType> &ranks)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably also check that these views have the correct rank (here and below).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants