@cjnolet A few questions/notes.

I think the libraft run: section should be using the "run" pinnings and not the "host" pinnings.

Should the cudatoolkit, cuda-profiler-api, and librmm dependencies be removed from host/run of libraft-headers as well? It sounds like the request is to have a header package that supplies none of its dependencies if those can be acquired via CPM and/or via the system.

Should the cudatoolkit, cuda-profiler-api, and librmm dependencies be removed from host/run of libraft-headers as well? It sounds like the request is to have a header package that supplies none of its dependencies if those can be acquired via CPM and/or via the system.

We shouldn't remove the entire cudatoolkit, but maybe separate it out into 1) just bare essential cudatoolkit dependency itself and 2) an additional section for the additional math libraries.

RAFT expects rmm and cudatoolkit at a bare minimum (though rmm will end up being downloaded automatically w/ cpm if they haven't been installed already), so, maybe we could remove rmm at least? The reality is that anyone building w/ the libraft headers should be using cmake, which is going to make sure these dependencies are there (everything other than cudatoolkit anyways), else it'll download them automatically. What are your thoughts here? Please guide...

cc @robertmaynard to make sure I did this properly. The idea is to make linking against the cuda math libraries optional for individual libraries after the libraft-headers conda package has been installed but pass them through raft::raft by default.

@cjnolet looks good from my side, thank you !
I'm still unsure whether we can remove the "run" dependency of libraft-headers from cugraph-ops, since this was added by the cugraph team (cugraph-ops being a module necessary to build/run cugraph). Similarly, I would argue that cugraph-ops should not have libraft-headers as a "run" dependency, but the cugraph team will have to decide, since they may have to update their CMake files as well.

@MatthiasKohl this PR should actually fix the dependencies for you. We are splitting the headers into a base "libraft-headers-impl" (which we might rename to be more concise) and "libraft-headers" has a dependency on the impl. The impl package removes the runtime dependency on the cuda math libraries and you can do the same in your get_raft in cmake. The explicit run dependency on the headers is only to stop cugraph-ops from trying to download them when building its own conda package so this should fix that whilst also fixing the math lib dependency.

Cugraph will have a host time dependency on library-headers but a runtime dependency on libraft-headers-impl. Does that make sense?

@cjnolet so IIUC, we will be able to make the impl package a host dependency on cugraph-ops, and remove the run dependency, right ?
This way, when building cugraph-ops, we should not download anything in cmake, but end users of cugraph-ops also don't need to download the additional dependencies from libraft-headers.
That would be ideal, thank you very much !

@cjnolet so IIUC, we will be able to make the impl package a host dependency on cugraph-ops, and remove the run dependency, right ?

You might be able to use EXCLUDE_FROM_ALL in get_raft.cmake to accomplish that, but it assumes that an end-user will not need raft at all in order to use cugraph-ops (e.g. you have not exposed any raft includes or apis on your own public api headers).

You might be able to use EXCLUDE_FROM_ALL in get_raft.cmake to accomplish that, but it assumes that an end-user will not need raft at all in order to use cugraph-ops (e.g. you have not exposed any raft includes or apis on your own public api headers).

You're right, we do have public APIs with raft structures, so we'd need at least the impl package as a run dependency.

@MatthiasKohl we have also deprecated our pointer-based APIs, which I notice you are using for rng. I'm going to prioritize changing over the rng APIs first so that they all accept raft::resources objects instead of raft::device_resources. raft::resources is a completely agnostic key/value store for resources (like cublas handles) and doesn't require any of the math libs to use. This way we can update cugraph-ops to use the new rng apis without polluting them with unecessary dependencies.

Making a note here that until all the APIs that are being compiled into libraft are using raft::resources instead of raft::device_resources, we will need to depend on all of the math libs.

@vyasr JFYI- @bdice and I did a huddle yesterday to iron out many of his initial comments (and to improve both of our understand- his from the cmake side and mine from the conda recipe / cudatoolkit packaging side).

@MatthiasKohl we have also deprecated our pointer-based APIs, which I notice you are using for rng. I'm going to prioritize changing over the rng APIs first so that they all accept raft::resources objects instead of raft::device_resources. raft::resources is a completely agnostic key/value store for resources (like cublas handles) and doesn't require any of the math libs to use. This way we can update cugraph-ops to use the new rng apis without polluting them with unecessary dependencies.

Thanks, that makes sense, we should be able to do this during 23.06 without issues.

The whole discussion here and the requirements people are bringing up make me think we should offer dev packages. Where the dev provides all the required bits to compile against, and the base package offers the smallest set.

The whole discussion here and the requirements people are bringing up make me think we should offer dev packages. Where the dev provides all the required bits to compile against, and the base package offers the smallest set.

@robertmaynard I think what you suggest here is a good state for us to get to (with the exception of rmm and cudatoolkit). I'm trying to minimize potential breakage downstream in the meantime with these changes so I've opted to keep the libraft-headers name for now. Given that any other users are going to need the ctk runtime and rmm at the very minimum, I don't foresee a problem w/ keeping these in both the cmake and conda.

/merge