-
-
Notifications
You must be signed in to change notification settings - Fork 4
ENH:TST Make changes needed to make xsf work as CuPy submodule and add proof of concept CuPy tests. #39
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
Conversation
I set up pixi to install dependencies and run the CuPy GPU tests and it works locally. I added a workflow file based on https://github.com/scipy/scipy/blob/main/.github/workflows/gpu-ci.yml, but it seems that those tests aren't getting picked up. |
@steppi I fixed an org-level configuration. The repos that the GPU runners are allowed to run on are whitelisted, it's |
Thanks @rgommers! How long does it usually take for the runner to get picked up? I see some on SciPy that have been waiting for over 11 minutes, so I guess it can take some time. |
I guess there's just one machine, and it's first come first serve. |
Beautiful! CuPy tests running and passing. Thanks @rgommers! |
🎉 Yes, it's a single machine indeed. On average it seems about 40% - 50% utilized, so at peak times there'll be a bit of a wait. |
c72343e
to
714dec2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This LGTM on a high level. It would be good to reduce duplication between the tests but that can be left for a followup. Let's first see how stable all of this is in CI.
Thanks @dschmitz89! The original tests in SciPy that #37 refers to avoided duplication, but I chose to reduplicate here to make it easier to run tests for specific functions through pytest. There's a way to keep things DRY while still allowing running tests for specific functions using custom pytest marks, and that's what I plan to do in the future. |
This PR makes a couple minor changes to
xsf/include/xsf/config.h
needed to makexsf
work as a submodule of CuPy.invoke_result
does not work in CuPy because CuPy still compiles its ufunc kernels with language standard C++11. When I make a PR to CuPy to add xsf as a submodule, I will also ask about increasing this, either by adding an ability to pass compiler flags incupy._core.create_ufunc
or updating the language standard incupy._core.create_ufunc
across the board. There are a number functions in or soon to be in xsf that have not been to CuPy yet and which make use of C++17 features, so this is needed for more than justinvoke_result
.config.h
was previously relying on a private symbol defined bylibcudacxx
to determine if compilation was being done with the CUDA jitNVRTC
. This symbol was removed in a recent version oflibcudacxx
. Changed to use__CUDACC_RTC__
.I've also added a proof of concept Python test file for testing xsf in CuPy. It would be nice configure Pixi to allow for installing the dependencies and running these tests. It would also be nice to set up GPU CI using this test file. Finishing this work would close #37.