Evovest / EvoTrees.jl

Boosted trees in Julia
https://evovest.github.io/EvoTrees.jl/dev/
Apache License 2.0
174 stars 20 forks source link

Make CUDA a weak dependency #226

Closed devmotion closed 8 months ago

devmotion commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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!