rapids_cpm_thrust installs to a location that won't be marked system #98
Conversation
robertmaynard
added
bug
|
@harrism @jrhemstad This closes the loop on the discussion on making sure RAPIDS always uses the newer version of thrust when being components. With |
|
@allisonvacanti I think you should be aware of this issue. |
This is horrifying 😅 The joys of being packaged with a toolchain.... Is there some way we could simplify this on Thrust's end? I really don't want ya'll to have to maintain a copy of our install rules. If you want to add a |
|
I think going with a |
|
#NVIDIA/thrust#1541 -- we'll need the same changes to the install rules in CUB (in |
So in my mind this is the way forward:
@allisonvacanti reasonable to you? |
Do you mean 1.14? This sounds good. I don't think you'd have any issues tracking later versions, too, since I don't expect our install rules to change anytime soon. I'll be releasing 1.15 next week. FYI, the upcoming releases are here. If the INFIX patch is ready by 12/6 we can ship it in 1.16, otherwise it'll wait until 1.17 goes out in February. |
Yes I meant 1.14 |
Projects such as cudf, and rmm require a newer versions of thrust than can be found in the oldest supported CUDA toolkit.This requires these components to install/packaged so that consumers use the same version. To make sure that the custom version of thrust is used over the CUDA toolkit version we need to ensure we always use an user include and not a system. By default if we allow thrust to install into `CMAKE_INSTALL_INCLUDEDIR` alongside rmm (or other pacakges) we will get a install tree that looks like this: ``` install/include/rmm install/include/cub install/include/thrust ``` This is a problem for CMake+NVCC due to the rules around import targets, and user/system includes. In this case both rmm and thrust will specify an include path of `install/include`, while thrust tries to mark it as an user include, since rmm uses CMake's default of system include. Compilers when provided the same include as both user and system always goes with system. Now while rmm could also mark `install/include` as system this just pushes the issue to another dependency which isn't built by RAPIDS and comes by and marks `install/include` as system. Instead the more reliable option is to make sure that we get thrust to be placed in an unique include path that to other project will use. In the case of rapids-cmake we install the headers to `include/rapids/thrust`.
More changes for CI style
robertmaynard
force-pushed
the
fea/use_custom_thrust_install_rules
branch
from
7b93c09
to
b7ba406
Compare
|
@gpucibot merge |
…1863) Originally this PR was about building FAISS shared libs via CPM, but @rlratzel mentioned cuGraph doesn't need FAISS anymore, and it'd be better if we remove it. If we need FAISS again in the future, we can add `faiss::faiss` back to `target_link_libraries` without needing extra CPM configuration as it will be available [via raft](rapidsai/raft#345). Edit: Removed more dependencies that we inherit from raft and/or rmm. Depends on rapidsai/raft#345. Edit 2: I think the CUDA 11.0 thrust issue will be solved by rapidsai/rapids-cmake#98 Authors: - Paul Taylor (https://github.com/trxcllnt) - Divye Gala (https://github.com/divyegala) Approvers: - Brad Rees (https://github.com/BradReesWork) - AJ Schmidt (https://github.com/ajschmidt8) URL: #1863
Projects such as cudf, and rmm require a newer versions of thrust than can
be found in the oldest supported CUDA toolkit.This requires these components to
install/packaged so that consumers use the same version. To make sure that the
custom version of thrust is used over the CUDA toolkit version we need to ensure
we always use an user include and not a system.
By default if we allow thrust to install into
CMAKE_INSTALL_INCLUDEDIRalongsidermm (or other pacakges) we will get a install tree that looks like this:
This is a problem for CMake+NVCC due to the rules around import targets, and
user/system includes. In this case both rmm and thrust will specify an
include path of
install/include, while thrust tries to mark it as an user include,since rmm uses CMake's default of system include. Compilers when provided the same include
as both user and system always goes with system.
Now while rmm could also mark
install/includeas system this just pushes the issue toanother dependency which isn't built by RAPIDS and comes by and marks
install/includeassystem.
Instead the more reliable option is to make sure that we get thrust to be placed in an
unique include path that to other project will use. In the case of rapids-cmake we install
the headers to
include/rapids/thrust.