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

staticcsrgraph: use device type instead of execution space to construct views #3991

Merged
merged 1 commit into from
May 1, 2021

Conversation

lucbv
Copy link
Contributor

@lucbv lucbv commented Apr 30, 2021

Using the execution space to construct views internally can lead to issues when the staticcrsgraph itself is built using a non default memory space.
This leads to build failures in Tpetra on AMD.
Closes issue #3990.

@dalg24-jenkins
Copy link
Collaborator

Can one of the admins verify this patch?

…ct views

Using the execution space to construct views internally can
lead to issues when the staticcrsgraph itself is built
using a non default memory space. This leads to build failures
in Tpetra on AMD. Closes issue kokkos#3990.

Applying clang-format
@masterleinad
Copy link
Contributor

Looks good to me. Pretty much what I would have done in a few minutes. 🙂
Your commit contains some tabs. Please make sure to fix the indentation with clang-format-8 (or just run scripts/apply_clang_format).

@lucbv lucbv force-pushed the hiphostpinned_create_staticcsrgraph branch from 5540e56 to 05901a8 Compare April 30, 2021 19:46
@lucbv
Copy link
Contributor Author

lucbv commented Apr 30, 2021

@masterleinad
I realized I had forgotten to apply clang-format and just performed a quick rebase, I think it should be fine now.

Copy link
Contributor

@masterleinad masterleinad left a comment

Choose a reason for hiding this comment

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

OK from my side but I certainly want someone else to confirm that this is fine.

@masterleinad
Copy link
Contributor

OK to test.

@dalg24
Copy link
Member

dalg24 commented Apr 30, 2021

OK to test

@lucbv
Copy link
Contributor Author

lucbv commented Apr 30, 2021

@masterleinad there is a build failing but the error is not pointing at Kokkos code explicitly and I do not have access to that machine, is there a simple way to reproduce this?
Or does it look like a fluke or a setup issue on the machine?

@dalg24
Copy link
Member

dalg24 commented May 1, 2021

Don't worry about that failure. We recently updated the CI container images and it has been failing since.

@dalg24 dalg24 merged commit 9b0772a into kokkos:develop May 1, 2021
dalg24 added a commit that referenced this pull request May 5, 2021
 [3.4.1] Take over #3991: staticcsrgraph: use device type instead of execution space to construct views
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.

None yet

5 participants