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

Cut down on the number of AccessTraits<Predicates, PredicatesTag> #978

Merged
merged 6 commits into from
Dec 23, 2023

Conversation

aprokop
Copy link
Contributor

@aprokop aprokop commented Dec 10, 2023

Follow up to #967.

@aprokop aprokop added the refactoring Code reorganization label Dec 10, 2023
@aprokop aprokop changed the title Cut down on the number of AccessTraits<Primitives, PredicatesTag> Cut down on the number of AccessTraits<Predicates, PredicatesTag> Dec 15, 2023
@aprokop
Copy link
Contributor Author

aprokop commented Dec 19, 2023

After spending quite a bit of time on it, I have no idea how to fix the CUDA 11.0.3 ptxas fatal error. Def a compiler bug, but can't figure out how to work around it.

@aprokop aprokop mentioned this pull request Dec 19, 2023
@masterleinad
Copy link
Collaborator

masterleinad commented Dec 19, 2023

It seems to complain about unresolved extern functions for

ArborX::AccessTraits<Kokkos::View<ArborX::Intersects<ArborX::Box>*, Kokkos::Device<Kokkos::Cuda, Kokkos::CudaSpace> >, ArborX::PredicatesTag, void>::get(Kokkos::View<ArborX::Intersects<ArborX::Box>*, Kokkos::Device<Kokkos::Cuda, Kokkos::CudaSpace> > const&, int)
ArborX::Intersects<ArborX::ExperimentalHyperGeometry::Sphere<3, float> >&& std::forward<ArborX::Intersects<ArborX::ExperimentalHyperGeometry::Sphere<3, float> > >(std::remove_reference<ArborX::Intersects<ArborX::ExperimentalHyperGeometry::Sphere<3, float> > >::type&)
ArborX::AccessTraits<Kokkos::View<ArborX::Intersects<ArborX::Sphere>*, Kokkos::Device<Kokkos::Cuda, Kokkos::CudaSpace> >, ArborX::PredicatesTag, void>::get(Kokkos::View<ArborX::Intersects<ArborX::Sphere>*, Kokkos::Device<Kokkos::Cuda, Kokkos::CudaSpace> > const&, int)

@aprokop
Copy link
Contributor Author

aprokop commented Dec 19, 2023

It seems to complain about unresolved extern functions for

Yeah, but it doesn't make sense to me.

src/details/ArborX_AccessTraits.hpp Outdated Show resolved Hide resolved
src/ArborX_DistributedTree.hpp Outdated Show resolved Hide resolved
InsertGenerator<FirstPassTag, PermutedPredicates, Callback, OutputView,
CountView, PermutedOffset>{callback, out, counts,
permuted_offset},
InsertGenerator<FirstPassTag, Callback, OutputView, CountView,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to specify the template parameters other than the tag?

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 tried removing them, and it failed. I always forget what the rules are.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If one template parameter is specified, all of them need to be provided, see https://godbolt.org/z/eKhnTW3nr. Aggregate deduction only works if no template parameters are given (which would require storing the tag here).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you.

@aprokop aprokop force-pushed the access_traits_proliferation_predicates branch from 71366f8 to 6931802 Compare December 20, 2023 14:29
@aprokop
Copy link
Contributor Author

aprokop commented Dec 20, 2023

Rebased on master to resolve conflicts with #970.

Copy link
Contributor

@dalg24 dalg24 left a comment

Choose a reason for hiding this comment

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

Good enough

@aprokop
Copy link
Contributor Author

aprokop commented Dec 21, 2023

Good enough

Thanks. Except it f-ing does not build on CUDA-11.0.3 because of ptxas and I have no idea how to fix that.

@aprokop
Copy link
Contributor Author

aprokop commented Dec 23, 2023

Given that I could not figure out how to fix cuda/11.0.3 build with nvcc, we decided to (hopefully temporarily) switch the cuda version in the CI to cuda/11.1 to make progress with refactoring. The hope is that after we complete the refactoring, we would try to switch back and maybe the issue would disappear. The goal is to not waste time on it right now.

The reason we went to 11.1 and not 11.4 is to not get excited with CTAD and then having to roll it back.

We actually lived without 11.0 build for quite some time in the beginning of the year. Had to reintroduce it when HACC team found issues in April.

Having said that, we would welcome any attempts to fix it.

@aprokop
Copy link
Contributor Author

aprokop commented Dec 23, 2023

CUDA 11.1.1 passes. SYCL is broken until #993 or #994 is fixed.

@aprokop aprokop merged commit 0a7cb09 into arborx:master Dec 23, 2023
0 of 2 checks passed
@aprokop aprokop deleted the access_traits_proliferation_predicates branch December 23, 2023 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code reorganization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants