-
Notifications
You must be signed in to change notification settings - Fork 41
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
Require Kokkos 4.1 #973
Require Kokkos 4.1 #973
Conversation
@@ -89,7 +89,7 @@ RUN SCRATCH_DIR=/scratch && mkdir -p ${SCRATCH_DIR} && cd ${SCRATCH_DIR} && \ | |||
rm -rf ${SCRATCH_DIR} | |||
|
|||
# Install Kokkos | |||
ARG KOKKOS_VERSION=4.0.00 |
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 not bump 4.0.00
to 4.1.00
and 4.1.00
to 4.2.00
?
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 would leap frog the versions. Change 4.0 to 4.2 now, change 4.1 to 4.3 next time. This way we have a mix of older builds and newer builds.
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.
For SYCL
in particular I would prefer using the most recent release. Otherwise, I don't have a strong opinion.
cda93d6
to
a9dba00
Compare
@@ -32,6 +31,7 @@ | |||
#include <ArborX_TraversalPolicy.hpp> | |||
|
|||
#include <Kokkos_Core.hpp> | |||
#include <Kokkos_Profiling_ScopedRegion.hpp> |
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 just realized it is not provided by <Kokkos_Core.hpp>
:(
#include <ArborX_DetailsKokkosExtArithmeticTraits.hpp> | ||
#include <ArborX_DetailsKokkosExtMinMaxOperations.hpp> |
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.
What is the deal with these? Oversight?
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.
Yes, probably happened during a rebase.
@@ -22,6 +23,9 @@ | |||
#include <ArborX_DetailsWeightedEdge.hpp> | |||
#include <ArborX_LinearBVH.hpp> | |||
|
|||
#include <Kokkos_Core.hpp> |
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.
OK
a9dba00
to
be2ae15
Compare
@@ -3,7 +3,7 @@ in the environment file. Its value will be prepended along with the service | |||
name to the container on start up. | |||
|
|||
|
|||
You may use multiple compose files to customize your container. For instance, you | |||
You may use multiple compose files to customize your container. For instance, you |
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 did you change? This is likely me using two spaces between sentences.
In any case, not really related to this PR. I am not requesting changes but if there is any further push I would like you to undo.
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 changed it in a9dba00, before your review, when I needed to force push to fix the incorrect Jenkins links, and was looking for a minor change.
be2ae15
to
c47f38f
Compare
No description provided.