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

Protecting lambda functions, in case cuda is enabled. #85

Merged
merged 2 commits into from
Sep 20, 2017
Merged

Protecting lambda functions, in case cuda is enabled. #85

merged 2 commits into from
Sep 20, 2017

Conversation

mndevec
Copy link
Contributor

@mndevec mndevec commented Sep 18, 2017

Should solve #72

crtrott
crtrott requested changes Sep 19, 2017
View reviewed changes
src/common/KokkosKernels_SparseUtils.hpp Outdated
@@ -1582,6 +1581,7 @@ void kk_create_incidence_tranpose_matrix_from_lower_triangle(
bool use_dynamic_scheduling = false,
bool chunksize = 4){

#ifndef KOKKOS_HAVE_CUDA
Copy link
Member

Choose a reason for hiding this comment

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

Rename to KOKKOS_ENABLE_CUDA

mhoemmen
mhoemmen approved these changes Sep 20, 2017
View reviewed changes
Copy link
Contributor

@mhoemmen mhoemmen left a comment

Choose a reason for hiding this comment

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

I think it's OK, but you may want to change the PR name (it's not about lambda functions, just about protecting CUDA-only code, right?).

@mndevec
Changing all KOKKOS_HAVE_CUDAs to KOKKOS_ENABLE_CUDA. This needs to b…
928d7e3 
…e done for OpenMP, Threads and Serial as well.
@mndevec
Copy link
Contributor Author

mndevec commented Sep 20, 2017

@mhoemmen
It is basically disabling the code that is written with lambdas when Cuda is enabled. This code is not used in any downstream.

@mndevec
Copy link
Contributor Author

mndevec commented Sep 20, 2017

I have changed the use of KOKKOS_HAVE_CUDA to KOKKOS_ENABLE_CUDA all over the package.
We will need to the same of OpenMP, Serial, and Pthreads.
@srajama1 @kyungjoo-kim @ambrad

@crtrott crtrott merged commit c6f5aaa into kokkos:develop Sep 20, 2017
@crtrott crtrott deleted the cuda_lambda_fix branch September 20, 2017 19:06
mhoemmen
mhoemmen reviewed Sep 21, 2017
View reviewed changes
Copy link
Contributor

@mhoemmen mhoemmen left a comment

Choose a reason for hiding this comment

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

I found something that looks weird -- see comment.

perf_test/graph/KokkosGraph_triangle.cpp
@@ -358,7 +358,7 @@ int main (int argc, char ** argv){
#endif


#if defined( KOKKOS_HAVE_CUDA1 )
#if defined( KOKKOS_ENABLE_CUDA1 )
Copy link
Contributor

Choose a reason for hiding this comment

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

What's "CUDA1"? Did you mean to type KOKKOS_ENABLE_CUDA instead?

Copy link
Member

Choose a reason for hiding this comment

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

Jup thats a typo we overlooked ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not a typo, it is intentional to disable the code.

@mhoemmen
Copy link
Contributor

@mndevec Argh, I should have looked harder: what is KOKKOS_ENABLE_CUDA1? Did you mean to type KOKKOS_ENABLE_CUDA instead?

@mndevec
Copy link
Contributor Author

mndevec commented Sep 21, 2017

@mhoemmen
No this code is disabled in purpose. I need to make couple more optimizations to run the code on GPUs.
For example, it is currently using parallel GNU sort.

@kokkos-devops-admin kokkos-devops-admin mentioned this pull request Mar 9, 2021
Merged
@kokkos-devops-admin kokkos-devops-admin mentioned this pull request Oct 5, 2021
Merged
This was referenced Aug 15, 2023
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhoemmen mhoemmen mhoemmen approved these changes

@crtrott crtrott crtrott approved these changes

@srajama1 srajama1 Awaiting requested review from srajama1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mndevec @mhoemmen @crtrott