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

Enable testing with SYCL #234

Merged
merged 7 commits into from
Jan 30, 2023
Merged

Enable testing with SYCL #234

merged 7 commits into from
Jan 30, 2023

Conversation

masterleinad
Copy link
Contributor

Corresponds to #223 but for SYCL.

@@ -181,9 +181,12 @@ void test_mdarray_ctor_data_carray() {
free_array(errors);
}

// host data can't be used in device code
Copy link
Contributor

@mhoemmen mhoemmen Jan 27, 2023

Choose a reason for hiding this comment

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

What host data does the test use? I'm guessing that the mdarray is defaulting to vector as its container; should we consider changing that to array so that the test can remain enabled, or does the test really depend on using vector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I basically meant std::vector. I pushed a commit using std::array in the test case instead which works with SYCL but I would guess not with Cuda or HIP.

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.

Thanks for this change!

At some point, after this PR merges : - ) , it would be excellent if we could refactor the testing infrastructure so that it could rely less on duplicated code protected by macros, and rely more on compile-time dispatch.

@dalg24
Copy link
Member

dalg24 commented Jan 28, 2023

Will conflict with #233

@crtrott
Copy link
Member

crtrott commented Jan 30, 2023

SYCL testing doesn't seem to work inside the runner. But that wasn't really the intend of the runner anyway. So I think we should just revert the change to the test config thing.

@crtrott crtrott merged commit fc4b692 into kokkos:stable Jan 30, 2023
mhoemmen added a commit to mhoemmen/mdspan that referenced this pull request Jul 26, 2023
This hopefully will prevent github from loading the cached
mdspan for PR kokkos#234.  The cached mdspan has the wrong tag.
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.

4 participants