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 a weak dependency #226

Closed
devmotion opened this issue May 24, 2023 · 3 comments · Fixed by #259
Closed

Make CUDA a weak dependency #226

devmotion opened this issue May 24, 2023 · 3 comments · Fixed by #259

Comments

@devmotion
Copy link
Contributor

I think it would be very useful for downstream packages that are mainly/only using the CPU functionality if the CUDA dependency would be made a weak dependency on Julia >= 1.9. That is, in newer Julia versions CUDA and all its dependencies would not be installed and loaded automatically but the GPU features would be loaded only if the user (or some other package loaded by the user) loads CUDA.

I had a quick look at the code, and it seems this would require some refactoring since such extensions are not supposed to (and can't really) export new structs but are only intended to extend existing functionality in the main package. Without any more detailed inspection, I think it might be feasible to unify the XX and XXGPU structs by adding type parameters for the contained vectors, matrices, etc. I also wonder if it would be helpful to dispatch on the types of X and y instead of supporting a device keyword argument - it seems this would make it easier to extend methods such as init_evotree by only defining additional methods for CuArrays in the extension.

What do you think? Would you consider reviewing and possibly merging a PR with such changes?

@jeremiedb
Copy link
Member

I think it would be an interesting development. It was part of my orignal intent to have the GPU mode to be more directly integrated with the CPU interface. I haven't looked much at Julia 1.9 yet, so I'm not familiar with weak dependency constraints, though I'd assume that, unlike structs, new functions are supported (since I don't see how to avoid GPU specific kernels functions)?

Although I agree most functions could dispatch on whehter inputs are Arrays vs CuArrays, I think the device parameter couldn't be dropped as I don't want the user to be forced to provide CuArrays as original inputs. Beyond being aligned with other libraries behavior such as XGBoost / LightGBM, I'm looking to have a first class DataFrame support, so categorical vectors are directly handled for example. I'd still want users to be able to train on GPU while providing a DataFrame as input, despite not having a GPU equivalent to DF.

In short, I'd be happy to to review and merge your contribution. Potential caveats would be about adverse impact regarding code maintainability or performance, but I wouldn't any of these considerations to be an issue here.

@jeremiedb
Copy link
Member

@devmotion For info, I'm currently actively working on a refactor to enable a DataFrames first support at https://github.com/Evovest/EvoTrees.jl/tree/streaming
It involves several changes to the underlying structs and I'll take the opportunity to further harmonise the CPU and GPU through dispatch. As such, I think it would be worth waiting to have this refactor settled prior to putting efforts on moving CUDA as an extension. I'm targeting to have something functional in about a week.

@jeremiedb
Copy link
Member

@devmotion v0.15.0 has just been released, all GPU structs are now removed, so I believe it should make the move of CUDA to weak dependency significantly easier!

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 a pull request may close this issue.

2 participants