Use _py-xgboost-mutex to differentiate libxgboost CPU vs GPU

[ksangeek]

I see that libxgboost conda package also provides the CLI for xgboost, but I don't see a method for a user to select a CPU or a GPU variant of this.
Should there be a meta-packages named libxgboost-cpu and libxgboost-gpu to help users selectively install the variant they desire?

And also maybe libxgboost should have a dependency on the selection package _py-xgboost-mutex so that the CPU and the GPU variants are not wrongly installed?

@jjhelmus Pulling you in here for comments, based on your comments in #14

[beckermr]

Ack. I thought we had this right.

[beckermr]

So conda does not ship the GPU variant.

We do depend on the mutex package to make sure the env is consistent. https://github.com/conda-forge/xgboost-feedstock/blob/master/recipe/meta.yaml#L70

Do you have an example of how this is going wrong?

[beckermr]

OK. So we cannot do this because the R package also depends on libxgboost. I am confused as to how you got an env to this state. You should not install libxgboost directly but instead you need to install one of the frontend packages that have the mutex dependencies.

[ksangeek]

I see that a user could install libxgboost directly and I don't see anything to restrict a user from doing that e.g. https://anaconda.org/conda-forge/libxgboost. I think you might be referring to that it is not common for a user to just install libxgboost conda package? Or am I missing something here?
My concern was only for the case when someone would just install libxgboost directly (just wants the xgboost CLI).

[beckermr]

Installing libxgboost directly is not how the package is supposed to be used, but yes of course people can do what they please.

I wonder if we should be separating the CLI from the lib package? Then we could use the mutex as it is supposed to work.

[ksangeek]

Yes, separating the CLI from libxgboost conda package would take care of the concern I brought up. Based on the discussion here, it seems that it is not expected for anyone to just install libxgboost and so is not a major concern anyway!
Thank You @beckermr for spending time on this.

[beckermr]

Happy to chat! I am going to leave this issue open for now.

[jakirkham]

Would it make sense to have a single mutex for all of these packages as they will all be dependent on libxgboost for CPU or GPU support?

[beckermr]

When you say "single mutex," what do you mean?

[jakirkham]

Sorry a single package to select GPU or CPU builds. Right now I see _py-xgboost-mutex and _r-xgboost-mutex. Am wondering if we can just have _libxgboost-mutex, which aligns C++, Python, and R. Does that make more sense?

[beckermr]

Ah ok. Does having a CPU python version installed in the env mean the GPU one cannot be? I don't know the internals well enough to understand the constraints here.

[jakirkham]

My guess is all packages using libxgboost need to be either CPU or GPU. That said, I don't know if the CPU/GPU parts extend outside of libxgboost or if they are contained within a common public APIs at the C++ level. If it's the latter, the Python and R bindings may be no different whether they are built to target CPU or GPU as only libxgboost would be affected. @RAMitchell, could you please provide us some guidance here?

[RAMitchell]

None of the Python code is different, the binary compiled for GPU use can safely be used on a system without a GPU. It will only fail if a user tries to call some GPU function and the cuda runtime does not exist.

[beckermr]

so we can just compile in gpu support for everything? why did we have the mutex in the first place? I copied it out of the main anaconda channels.

[jakirkham]

Do you happen to remember the author of these changes (to the recipe)? Maybe we can ask them?

[beckermr]

looks like it was @jjhelmus and/or @mingwandroid

[ksangeek]

There are a few things I'd like to add here -

1. From a conda packaging perspective, the GPU variant would need to depend on cudatoolkit for build and run as used here -
   https://github.com/AnacondaRecipes/xgboost-feedstock/blob/885f8b2583def2146d494254d04ce05456704d3b/recipe/meta.yaml#L47
   But the CPU variant does not need that!

2. I have seen the objects built for libxgboost GPU variant (which includes objects for CUDA code) make the library very bulky. So, regardless of point 1. I think it is still a good idea to have separate conda packages for the GPU and CPU variant.
   Ref -
   https://github.com/AnacondaRecipes/xgboost-feedstock/blob/885f8b2583def2146d494254d04ce05456704d3b/recipe/build.sh#L16-L19

While I had opened this issue, the idea was very close to what @jakirkham brought up here wrt to R. But my request was to include the dependency of - {{ pin_subpackage('_py-xgboost-mutex', exact=True) }} in the run/host section of libxgboost package, so that the CPU and GPU versions are exclusive. Installing the GPU variant (on a system where CPU variant is already installed) should prompt to clear off the conflicting packages (which includes libxgboost). In the current form, this recipe will not prompt to remove libxgboost.

xgboost-feedstock/recipe/meta.yaml

Lines 36 to 49 in 521defb

	outputs:
	- name: libxgboost
	script: install-libxgboost.sh
	requirements:
	build:
	- {{ compiler('c') }}
	- {{ compiler('cxx') }}
	- git
	- make
	- llvm-openmp >=4.0.1 # [osx]
	host:
	- llvm-openmp >=4.0.1 # [osx]
	run:
	- llvm-openmp >=4.0.1 # [osx]

And as @jakirkham brought up I am supportive of the idea to have a single mutex _xgboost-mutex, which aligns C++, Python, and R.

[mingwandroid]

If the gpu xgboost works just as well without gpus as the non-gpu xgboost then there's no good enough reason for the cpu variant to exist.

Let's discard all idea of mutexes then?

[jakirkham]

Personally would be fine with either keeping one mutex or dropping.

It sounds to me like @ksangeek's concerns are basically about total install size, which seems like a valid concern to me (note: this does come up in other contexts). Do we have a sense of what the size difference is now?

[jakirkham]

Have added PR ( #35 ) to try and simplify CPU/GPU selection based on the discussion here. Please take a look and share your thoughts. 🙂

[jakirkham]

We've since added GPU/CPU packages here. There may very well still be work needed, but please try it out and let us know

[hcho3]

@jakirkham Can we close this out now?

[jakirkham]

Yes this is done

If we want to make other changes, we should start a new issue discussing those