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.

Sign up for GitHub

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

Jump to bottom

Allow assignment of LayoutLeft to LayoutRight or vice versa for rank-0 Views #594

Closed
mhoemmen opened this issue Jan 3, 2017 · 4 comments
Closed

Allow assignment of LayoutLeft to LayoutRight or vice versa for rank-0 Views #594

mhoemmen opened this issue Jan 3, 2017 · 4 comments
Assignees
@hcedwar
Labels
Enhancement Improve existing capability; will potentially require voting
Milestone
2017 October

Comments

@mhoemmen
Copy link
Contributor

mhoemmen commented Jan 3, 2017

This works for rank-1 Views, but not for rank-0 Views (where it equally would make sense to work). This is not urgent for me, but it would be helpful (e.g., would reduce the need for some template parameters in my fix for trilinos/Trilinos#957 ). Thanks!
mhoemmen pushed a commit to trilinos/Trilinos that referenced this issue Jan 3, 2017
Tpetra: Let iallreduce take in & out Views of different layouts
06dc8a3 
@trilinos/tpetra This will help with #957.
Tpetra::Details::iallreduce used to require that both the input and
the output buffers have the same array_layout.  This was OK for rank-1
Views, because Kokkos allows assignment of LayoutLeft to LayoutRight
or vice versa for rank-1 Views.  However, Kokkos does NOT (currently)
allow assignment of LayoutLeft to LayoutRight or vice versa for rank-0
Views, even though this would make sense.  See the outstanding Kokkos
issue:

kokkos/kokkos#594

This commit adds template parameters for the array layouts of the send
and receive buffers, to iallreduce.  This lets the send and receive
buffers have different layouts.  In theory, only LayoutLeft and
LayoutRight should work, though we do not currently check this.  (I'm
not sure if LayoutStride makes sense for a rank-0 View.)
@hcedwar hcedwar added this to the Backlog milestone Jan 11, 2017
@hcedwar hcedwar added the Enhancement Improve existing capability; will potentially require voting label Jan 11, 2017
@mndevec
Copy link

mndevec commented Aug 4, 2017

What is the status of this issue?
We will probably need this fix for the integration of KokkosKernels into Trilinos by August 20.
Current ETI system in KokkosKernels casts all rank=0, rank=1 views to layoutleft, and integration is currently failing. Would this be an easy fix?
@crtrott
@srajama1

@srajama1
Copy link

srajama1 commented Aug 4, 2017

I didn't realize this. @hcedwar The urgency comes from a deliverable for ATDM deliverable to applications by August 20. Can we prioritize this ?

@ibaned ibaned modified the milestones: 2017-September (mid), Backlog Aug 4, 2017
@mndevec mndevec mentioned this issue Aug 4, 2017
Closed
@hcedwar hcedwar assigned hcedwar and unassigned crtrott Aug 7, 2017
hcedwar added a commit to hcedwar/kokkos that referenced this issue Aug 7, 2017
@hcedwar
Fix kokkos#594 , allow assignment of rank-0 views with different layo…
40bbc95 
…uts.
@mndevec
Copy link

mndevec commented Aug 7, 2017

@hcedwar
Replicating the error might be a bit difficult. It requires the snapshat of kokkoskernels develop branch into Trilinos, and the changes to Trilinos in https://github.com/mndevec/Trilinos/tree/kk_integration .

Let me run the tests again after kokkos commit, and I will let you know if the problem continues.

@hcedwar hcedwar mentioned this issue Aug 7, 2017
Merged
crtrott added a commit that referenced this issue Aug 7, 2017
@crtrott
Merge pull request #1017 from hcedwar/develop
d1eb7e8 
Fix #594 and add #990
@mndevec
Copy link

mndevec commented Aug 8, 2017 •
edited

@hcedwar
Thank you for the fix. The compilation issues seem to be solved.

@ibaned ibaned closed this as completed Aug 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hcedwar hcedwar

Labels
Enhancement Improve existing capability; will potentially require voting
Projects
None yet
Milestone
2017 October
Development

No branches or pull requests
6 participants
@mhoemmen @ibaned @hcedwar @crtrott @srajama1 @mndevec