-
Notifications
You must be signed in to change notification settings - Fork 407
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
#3785: Add execution space arguments for constructor overloads that might allocate a new underlying View #3904
#3785: Add execution space arguments for constructor overloads that might allocate a new underlying View #3904
Conversation
Can one of the admins verify this patch? |
I neglected that these constructor overloads were used in EMPIRE |
Could I please get a review on this @dalg24 @DavidPoliakoff @masterleinad @crtrott ? |
OK to test |
364e97d
to
6871d8e
Compare
…that might allocate a new underlying View
6871d8e
to
c3f7247
Compare
Ok, I'll make that revision. It makes the calling code simpler too.
|
d2cfd26
to
f8918ec
Compare
f8918ec
to
39e217e
Compare
Will this make it into the 3.4 release? |
It's negotiable. I suppose it means you would like us to include this patch in the release? |
yes - if possible. Short term, we will be adding it an application's internal fork of trilinos/kokkos. But we don't want to be out of sync from the releases for too long. Too easy to introduce incompatibility issues. |
Ping - anything else needed from me before this gets reviewed and merged? |
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.
Looks OK to me.
…that might allocate a new underlying View Patched from the upstream kokkos/kokkos repository into EM-Plasma/Trilinos by request of @pbmille and @rppawlo. See kokkos/kokkos#3904.
Patching Kokkos#3904 into EM-Plasma/Trilinos ahead of the next Kokkos release. See kokkos/kokkos#3904. * kokkos-3785: Kokkos#3785: Add execution space arguments for constructor overloads that might allocate a new underlying View
Follow-on to #3786