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 CUDA optional dependency #20

Merged
merged 15 commits into from
Oct 23, 2023
Merged

Make CUDA optional dependency #20

merged 15 commits into from
Oct 23, 2023

Conversation

pelesh
Copy link
Collaborator

@pelesh pelesh commented Oct 20, 2023

This PR reduces internal dependencies on CUDA SDK libraries. Classes MatrixHandler and VectorHandler are refactored to allow CPU-only or CUDA implementation of matrix and vector operations to be selected at runtime. With these changes:

  • ReSolve can be built without CUDA
  • Hardware backend (CPU or CUDA) is selected at runtime
  • The code now builds on Linux and MacOS
  • Fixed bug in FGMRES destructor

The build configuration allows for multiple hardware backends to coexist. In theory, we should be able to add HIP backend and build it at the same time as CUDA backend. If no GPU backend is built, a dummy device backend needs to be built instead (see CpuMemory.hpp).

Known issues not addressed in this PR:

  • matvec function implemented in MatrixHandlerCuda class corrupts matrix descriptor object.
  • There are memory leaks in unit tests for CUDA implementations of matrix and vector handlers as CUDA workspace is not deleted.

@pelesh pelesh added the enhancement New feature or request label Oct 20, 2023
@pelesh pelesh added this to the Hackathon milestone Oct 20, 2023
@cameronrutherford
Copy link
Collaborator

@pelesh PNNL GitHub supports MacOS and Windows runners. Can we add that for ReSolve in GitHub actions?

Additionally, now ReSolve supports building on Linux w/ Spack in GitHub actions, so we should add a pipeline and testing for those builds without CUDA.

@pelesh pelesh modified the milestones: Hackathon, First Release Oct 20, 2023
Copy link
Collaborator

@kswirydo kswirydo left a comment

Choose a reason for hiding this comment

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

I thought about it for some time and there was probably an easier solution to make all lin alg operatios (in VectorHandler and MatrixHandler) run with various backends, but it would had been a CMake nightmare and not necessarily backward compatible. I think the stuff in this PR is pretty good. I had some minor comments. Also, why IndexPlusValue changed its name?

examples/r_KLU_KLU_standalone.cpp Outdated Show resolved Hide resolved
examples/r_KLU_KLU_standalone.cpp Show resolved Hide resolved
examples/r_KLU_KLU_standalone.cpp Outdated Show resolved Hide resolved
resolve/vector/VectorHandler.cpp Show resolved Hide resolved
tests/functionality/testKLU_Rf.cpp Show resolved Hide resolved
Copy link
Collaborator

@cameronrutherford cameronrutherford left a comment

Choose a reason for hiding this comment

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

With better docs will come enlightenment. LGTM

EDIT: We should make issues for:

  • Future work
  • Known bugs
  • CI/CD additions

resolve/matrix/MatrixHandler.cpp Show resolved Hide resolved
resolve/vector/CMakeLists.txt Show resolved Hide resolved
resolve/workspace/CMakeLists.txt Show resolved Hide resolved
resolve/workspace/LinAlgWorkspace.hpp Show resolved Hide resolved
tests/functionality/testKLU.cpp Show resolved Hide resolved
@ryandanehy
Copy link
Collaborator

@pelesh PNNL GitHub supports MacOS and Windows runners. Can we add that for ReSolve in GitHub actions?

Additionally, now ReSolve supports building on Linux w/ Spack in GitHub actions, so we should add a pipeline and testing for those builds without CUDA.

Will that mean we have to setup a mirror to a PNNL github given this under ORNL?

@ryandanehy ryandanehy closed this Oct 23, 2023
@pelesh pelesh reopened this Oct 23, 2023
@pelesh
Copy link
Collaborator Author

pelesh commented Oct 23, 2023

@pelesh PNNL GitHub supports MacOS and Windows runners. Can we add that for ReSolve in GitHub actions?
Additionally, now ReSolve supports building on Linux w/ Spack in GitHub actions, so we should add a pipeline and testing for those builds without CUDA.

Will that mean we have to setup a mirror to a PNNL github given this under ORNL?

I think so.

@pelesh pelesh merged commit 395267b into develop Oct 23, 2023
2 checks passed
@cameronrutherford
Copy link
Collaborator

@pelesh PNNL GitHub supports MacOS and Windows runners. Can we add that for ReSolve in GitHub actions?
Additionally, now ReSolve supports building on Linux w/ Spack in GitHub actions, so we should add a pipeline and testing for those builds without CUDA.

Will that mean we have to setup a mirror to a PNNL github given this under ORNL?

Mirror from ORNL GitHub to PNNL GitLab should be sufficient without PNNL GitHub in between

@pelesh pelesh deleted the no-cuda-dev branch October 30, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants