Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rapids_cpm_bs_thread_pool() #651

Merged
merged 5 commits into from
Jul 23, 2024

Conversation

KyleFromNVIDIA
Copy link
Contributor

Add a function to download and export bshoshany/thread-pool.

Description

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@KyleFromNVIDIA KyleFromNVIDIA requested a review from a team as a code owner July 22, 2024 20:36
@KyleFromNVIDIA KyleFromNVIDIA added feature request New feature or request non-breaking Introduces a non-breaking change labels Jul 22, 2024
Add a function to download and export bshoshany/thread-pool.
@KyleFromNVIDIA KyleFromNVIDIA changed the base branch from branch-24.10 to branch-24.08 July 22, 2024 20:46
Copy link
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KyleFromNVIDIA
Copy link
Contributor Author

Done

rapids-cmake/cpm/bs_thread_pool.cmake Outdated Show resolved Hide resolved
rapids-cmake/cpm/bs_thread_pool.cmake Show resolved Hide resolved
docs/packages/packages.rst Show resolved Hide resolved
KyleFromNVIDIA added a commit to KyleFromNVIDIA/rapids-cmake that referenced this pull request Jul 22, 2024
@robertmaynard
Copy link
Contributor

It would be great to have a kvikio PR that leverages this work so we can validate we didn't miss anything

@KyleFromNVIDIA
Copy link
Contributor Author

rapidsai/kvikio#408 was already working on this, I'll update it to use this PR instead.

@KyleFromNVIDIA
Copy link
Contributor Author

Updated rapidsai/kvikio#408 to use this PR.

@KyleFromNVIDIA
Copy link
Contributor Author

Also opened rapidsai/cudf#16360 to test this PR.

@madsbk
Copy link
Member

madsbk commented Jul 23, 2024

Compiling twice fails in KvikIO now:

Executing cmake for libkvikio...
-- Found cuFile Batch API: TRUE
-- Found cuFile Stream API: TRUE
-- CPM: Using local package [email protected]
CMake Error at cmake/thirdparty/get_thread_pool.cmake:18 (include):
  include could not find requested file:

    /home/mkristensen/repos/kvikio/cpp/build/_deps/rapids-cmake-src/rapids-cmake/cpm/bs_thread_pool.cmake
Call Stack (most recent call first):
  cmake/thirdparty/get_thread_pool.cmake:25 (find_and_configure_thread_pool)
  CMakeLists.txt:84 (include)


-- Configuring incomplete, errors occurred!

@robertmaynard
Copy link
Contributor

Compiling twice fails in KvikIO now:

Executing cmake for libkvikio...
-- Found cuFile Batch API: TRUE
-- Found cuFile Stream API: TRUE
-- CPM: Using local package [email protected]
CMake Error at cmake/thirdparty/get_thread_pool.cmake:18 (include):
  include could not find requested file:

    /home/mkristensen/repos/kvikio/cpp/build/_deps/rapids-cmake-src/rapids-cmake/cpm/bs_thread_pool.cmake
Call Stack (most recent call first):
  cmake/thirdparty/get_thread_pool.cmake:25 (find_and_configure_thread_pool)
  CMakeLists.txt:84 (include)


-- Configuring incomplete, errors occurred!

This is due to how rapidsai/kvikio#408 changes are structured. The setting of the rapids-cmake-repo* variables need to happen on all evaluations of cpp/cmake/rapids_config.cmake

@KyleFromNVIDIA
Copy link
Contributor Author

Fixed the variable issue at rapidsai/kvikio#408.

@robertmaynard
Copy link
Contributor

Approving, presuming that the cudf and kvikio CI won't show any issues

@KyleFromNVIDIA
Copy link
Contributor Author

rapidsai/kvikio#408 is now passing CI

@KyleFromNVIDIA
Copy link
Contributor Author

rapidsai/cudf#16360 is passing CI too. Ready to merge?

@KyleFromNVIDIA
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 3962771 into rapidsai:branch-24.08 Jul 23, 2024
18 checks passed
rapids-bot bot pushed a commit that referenced this pull request Jul 24, 2024
Noticed while working on #651.

Authors:
  - Kyle Edwards (https://github.com/KyleFromNVIDIA)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants