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

Make sure cuda lambda support is enabled for cusparse tests #640

Merged
merged 1 commit into from
Mar 10, 2020

Conversation

jjwilke
Copy link
Contributor

@jjwilke jjwilke commented Mar 5, 2020

No description provided.

@ndellingwood
Copy link
Contributor

I think the right thing is to guard the tests themselves regarding lambda support, rather than erroring out at configure when cusparse is enabled and if cuda lambda support is not enabled.
I reproduced the error and submitting a PR with a guard on the test that fixes the error.

@jjwilke
Copy link
Contributor Author

jjwilke commented Mar 5, 2020

Yeah, we can definitely do that. The error is currently only if the test is also enabled. The concern would be silently turning off tests that people were potentially expecting to run. Do we want to warn when certain tests are inactive, but could/should be?

@ndellingwood
Copy link
Contributor

Do we want to warn when certain tests are inactive, but could/should be?

@jjwilke yes, that would be great. And I agree with your point about silently turning off tests. For the case that brought this issue up, Luc is replacing the lambda with a functor, but in general when we add tests would we be able to "register" them with the build system as using lambdas so the warning or configure error gets thrown if configured improperly? This would be awesome.

@lucbv
Copy link
Contributor

lucbv commented Mar 5, 2020

@ndellingwood @jjwilke
I think the right thing to do here is to add appropriate guards/warning in the file: perf_test/sparse/CMakeLists.txt at line 32.
This is much more focused compared to enabling lambda's for all sparse tests. Also I believe the PR might not be necessary after PR #618 is merged.

@ndellingwood ndellingwood merged commit c97616e into kokkos:develop Mar 10, 2020
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.

3 participants