[REVIEW] fetch thrust/cub from github

[rongou]

Use FetchContent to obtain thrust/cub release 1.9.0. This is needed to get per-thread default stream to work properly. Counterpart to rapidsai/rmm#378.

Closes #5346

@harrism @jrhemstad @mt-jones

[GPUtester]

Please update the changelog in order to start CI tests.

View the gpuCI docs here.

[rongou]

@harrism @jrhemstad these errors seem to be caused by gcc-5. Building with gcc-7 works, at least for me.

[kkraus14]

@harrism @jrhemstad these errors seem to be caused by gcc-5. Building with gcc-7 works, at least for me.

We can't just remove gcc-5 support as that's the default compiler shipped with Ubuntu 16.04 for example.

[rongou]

Let me dig through the code. Maybe we can do something on our end to fix this.

[harrism]

Closes #5346

[rongou]

rerun tests

[rongou]

@harrism I've been seeing the build timed out error a lot. Is it possible to increase the timeout value?

[kkraus14]

@harrism I've been seeing the build timed out error a lot. Is it possible to increase the timeout value?

No, that typically means that a webserver or something of the like is being flaky and we don't want a runaway job sitting there fighting with it.

[harrism]

Thanks for fixing the Cython paths!

[kkraus14]

Requesting changes to block merging until we're aligned on behavior regarding if someone comes in with some code built against the toolkit shipped Thrust / CUB version.

[rongou]

Requesting changes to block merging until we're aligned on behavior regarding if someone comes in with some code built against the toolkit shipped Thrust / CUB version.

Since thrust/cub are header-only libraries, code built with different thrust/cub versions is insulated from cuDF. Assuming that code is tested in its own environment, as cuDF is tested against the version of thrust/cub here, there shouldn't be a problem as there are no runtime dependencies on different versions.

[kkraus14]

Since thrust/cub are header-only libraries, code built with different thrust/cub versions is insulated from cuDF. Assuming that code is tested in its own environment, as cuDF is tested against the version of thrust/cub here, there shouldn't be a problem as there are no runtime dependencies on different versions.

This is only true if thrust types aren't exposed in our APIs. What if we have an API that takes a thrust::device_vector in the future and the signature of thrust::device_vector changes between toolkit version and the version we're using? What would be the behavior of a library returning its thrust::device_vector and trying to pass it into a libcudf API that expects to take in a newer version of thrust::device_vector.

[harrism]

This is only true if thrust types aren't exposed in our APIs. What if we have an API that takes a thrust::device_vector in the future and the signature of thrust::device_vector changes between toolkit version and the version we're using? What would be the behavior of a library returning its thrust::device_vector and trying to pass it into a libcudf API that expects to take in a newer version of thrust::device_vector.

(We already have such APIs, but they are mostly detail APIs. The single exception is cudf::make_strings_column() but that is arguably a detail API too.)

device_vector is a class template, so the types are specified at compile time. If there is a conflict, I would expect it to result in compile-time errors, because a user can't use any libcudf APIs that take or return device_vector without compiling those calls themselves.

While there could be problems here, I think the same problem would happen with header-only libraries that are NOT part of the CUDA Toolkit. For example if someone linked librmm into an application that also has its own use of spdlog. Or if they used a different implementation of STL. I think your fear is healthy, but I don't think it should stop us from going forward.

[kkraus14]

Just wanted to make sure that the limitations / possible issues were at least given some thought and discussed. Seems like we're okay with the tradeoff so approved. 😄

[rongou]

C++ dependency management certainly can be tricky. At Google the solution is to build everything from source, each library is versioned, even the toolchains are versioned (gcc/clang/nvcc etc.). This is the philosophy behind bazel. But to make it work at scale, you really need a distributed build system, content-addressable object store, distributed CI, etc. Otherwise everyone would be always waiting for the build to finish on their desktop (it already takes some time for cuDF :). Without that kind of infrastructure, I guess all you can do is to be very careful. When you pull in a binary dependency, you need to make sure it's actually compatible.

[kkraus14]

@mt-jones is working on reworking the CMake build infrastructure across all of the RAPIDS libraries to be able to better control this, but for now we can just be careful 😄