Skip to content
This repository has been archived by the owner on Mar 21, 2024. It is now read-only.

Allow Thrust and CUB headers to be nested in a custom directory #1541

Closed
alliepiper opened this issue Oct 18, 2021 · 3 comments · Fixed by NVIDIA/cub#415 or #1584
Closed

Allow Thrust and CUB headers to be nested in a custom directory #1541

alliepiper opened this issue Oct 18, 2021 · 3 comments · Fixed by NVIDIA/cub#415 or #1584
Assignees
Labels
helps: rapids Helps or needed by RAPIDS. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. P2: nice to have Desired, but not necessary. type: enhancement New feature or request.
Milestone

Comments

@alliepiper
Copy link
Collaborator

RAPIDS recently ran into a bug where the Thrust headers from a custom install were hidden by the Thrust headers in CTK, despite following the usual advice of including non-CTK Thrust headers with -I to override the implicit -isystem <CTK_INCLUDE_PATH>.

This is due to the custom include prefix being used by multiple projects, some of which are -isystem, and apparently compilers will use -isystem when the same path is specified with both -I and -isystem. A more detailed description of the issue is in rapidsai/rapids-cmake#98.

Currently, rapids-cmake is working around this by reproducing a copy our install rules, adjusted so that thrust/cub headers are installed to <prefix>/include/thrust/thrust and <prefix>/include/cub/cub.

We should provide a more robust solution, since it's likely that other users will also encounter this issue, and I don't want to break rapids if our install rules change. We should add some CMake variables to specify this extra directory, something like THRUST_INSTALL_HEADER_INFIX=thrust and CUB_INSTALL_HEADER_INFIX=cub, or something similar (I'm not sure if there's a more idiomatic way to say "infix" in CMake).

@alliepiper alliepiper added this to the 1.16.0 milestone Oct 25, 2021
@alliepiper alliepiper added type: enhancement New feature or request. helps: rapids Helps or needed by RAPIDS. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. P2: nice to have Desired, but not necessary. labels Oct 25, 2021
@neoblizz
Copy link

I think I am running into the same issue when compiling my project on Windows. My workflow fetches thrust's latest version (1.15.0), and then finds the cub includes within it.

https://github.com/gunrock/essentials/blob/master/cmake/FetchThrustCUB.cmake#L21-L22
I am wondering if this is the right way to do things though.

set(THRUST_INCLUDE_DIR "${thrust_SOURCE_DIR}")
set(CUB_INCLUDE_DIR "${thrust_SOURCE_DIR}/cub")

And then finally, I link to those includes for my project; https://github.com/gunrock/essentials/blob/master/CMakeLists.txt#L89-L92

target_include_directories(essentials
    INTERFACE include
    INTERFACE ${THRUST_INCLUDE_DIR}
    INTERFACE ${CUB_INCLUDE_DIR}
    INTERFACE ${MODERNGPU_INCLUDE_DIR}
	  INTERFACE ${RAPIDJSON_INCLUDE_DIR}
    INTERFACE ${CMAKE_CUDA_TOOLKIT_INCLUDE_DIRECTORIES}
)

Now, the first complaint on windows was (full log)

D:\a\essentials\essentials\externals\thrust-src\thrust/system/cuda/config.h(79): fatal error C1189: #error:  The version of CUB in your include path is not compatible with this release of Thrust. CUB is now included in the CUDA Toolkit, so you no longer need to use your own checkout of CUB. Define THRUST_IGNORE_CUB_VERSION_CHECK to ignore this. [D:\a\essentials\essentials\build\examples\sssp\sssp.vcxproj]

Which, I think it is trying to link an incorrect CUB version. What's weird is my actions is trying to compile with the latest cuda version, which I thought would mean equivalent latest thrust/cub versions, but I guess that is not a thing.

I tried fixing it by defining THRUST_IGNORE_CUB_VERSION_CHECK in my cmake for windows: https://github.com/gunrock/essentials/blob/master/CMakeLists.txt#L76,

target_compile_definitions(essentials
  INTERFACE 
    SM_TARGET=${ESSENTIALS_ARCHITECTURES}
    # For some reason, Thrust/CUB version mismatch on windows.
    $<$<PLATFORM_ID:Windows>:THRUST_IGNORE_CUB_VERSION_CHECK=1>
)

but now, I see: (full log)

D:\a\essentials\essentials\externals\thrust-src\cub\block\../block/block_exchange.cuh(36): fatal error C1083: Cannot open include file: '../config.cuh': No such file or directory [D:\a\essentials\essentials\build\examples\sssp\sssp.vcxproj]

@alliepiper
Copy link
Collaborator Author

set(CUB_INCLUDE_DIR "${thrust_SOURCE_DIR}/cub")

This is likely the main issue -- that cub symlink is a problematic legacy remnant since Windows doesn't support symlinks, which explains why Windows is giving you issues. The CUB headers are not reachable with those include paths on that OS, so it's pulling them from the CTK instead of the git repo.

For cross-platform projects, the CUB include path should be ${thrust_SOURCE_DIR}/dependencies/cub, as described here.

That said...there is a better way :)

I am wondering if this is the right way to do things though.

We have CMake packages that provide easier ways to use Thrust and CUB from other CMake projects. See this file for details. If you configure a Thrust target to use CUDA using thrust_create_target, it will automatically bring in the correct CUB headers, too.

You may also want to look into the CMakePackageManager, which provides a nicer interface around FetchContent.

What's weird is my actions is trying to compile with the latest cuda version, which I thought would mean equivalent latest thrust/cub versions, but I guess that is not a thing.

The version of Thrust/CUB shipped with CTK is usually a couple of versions behind what's available on Github. You can find out which version of Thrust/CUB is in your CTK by referencing this table.

I tried fixing it by defining THRUST_IGNORE_CUB_VERSION_CHECK

This is unlikely to work reliably. Thrust and CUB are tightly coupled these days, and different versions of the libraries are often incompatible with each other.

Your best bet is to use the CMake packages -- those are configured to avoid most of these issues.

@neoblizz
Copy link

Sweet, thank you for the resources! That solved my issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
helps: rapids Helps or needed by RAPIDS. only: cmake CMake changes only. Doesn't need internal NVIDIA CI. P2: nice to have Desired, but not necessary. type: enhancement New feature or request.
Projects
None yet
3 participants