Skip to content

build(cudf): Bring in ucxx when ucx is present and update all RAPIDS dependencies to latest#17572

Open
zoltan wants to merge 6 commits into
facebookincubator:mainfrom
zoltan:build/ucxx-fetch-when-ucx-available
Open

build(cudf): Bring in ucxx when ucx is present and update all RAPIDS dependencies to latest#17572
zoltan wants to merge 6 commits into
facebookincubator:mainfrom
zoltan:build/ucxx-fetch-when-ucx-available

Conversation

@zoltan
Copy link
Copy Markdown
Collaborator

@zoltan zoltan commented May 20, 2026

When VELOX_ENABLE_CUDF is on and system UCX (libucp + headers) is detected, declare and fetch ucxx alongside the other rapids dependencies. Nothing links against ucxx yet upstream; the wiring prepares the ground for a future ucx-exchange consumer.

Also bumps the default UCX version in setup-centos-adapters.sh from 1.19.0 to 1.20.1 (with matching docstring) to align with the version shipped in our dependency images.

ucxx is pinned to commit 8d47a9f (release v0.49.00) and uses the same commit-archive URL pattern as the other rapids deps so the update-cudf-deps.sh tooling can be extended uniformly.

When VELOX_ENABLE_CUDF is on and system UCX (libucp + headers) is
detected, declare and fetch ucxx alongside the other rapids
dependencies. Nothing links against ucxx yet upstream; the wiring
prepares the ground for a future ucx-exchange consumer.

Also bumps the default UCX version in setup-centos-adapters.sh from
1.19.0 to 1.20.1 (with matching docstring) to align with the version
shipped in our dependency images.

ucxx is pinned to commit 8d47a9f (release v0.49.00) and uses the same
commit-archive URL pattern as the other rapids deps so the
update-cudf-deps.sh tooling can be extended uniformly.
@netlify
Copy link
Copy Markdown

netlify Bot commented May 20, 2026

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 226990b
🔍 Latest deploy log https://app.netlify.com/projects/meta-velox/deploys/6a2052b2c1c510000879909a

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 20, 2026
@zoltan
Copy link
Copy Markdown
Collaborator Author

zoltan commented May 20, 2026

@bdice as discussed, this brings in ucxx as a dependency

FYI @dan13bauer

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 20, 2026

Build Impact Analysis

No build targets affected by this change.


Slow path • Graph generated from PR branch

@pedroerp pedroerp requested a review from bdice May 26, 2026 17:14
@majetideepak majetideepak requested a review from czentgr May 27, 2026 14:43
Comment thread CMake/resolve_dependency_modules/cudf.cmake
@majetideepak majetideepak requested a review from karthikeyann May 27, 2026 14:51
Copy link
Copy Markdown
Collaborator

@karthikeyann karthikeyann left a comment

Choose a reason for hiding this comment

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

Same comment as deepak's comment.
<LIB>_FOUND is a common pattern in cmake. consider using UCX_FOUND at 3 places.

Copy link
Copy Markdown
Collaborator

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I recommend adopting UCX_FOUND before merging.

I also filed zoltan#1 which includes ucxx in the cuDF dependency update script and bumps to the latest commits from release/26.06. Feel free to include one or both of those commits in this PR if you like, or we can do that in a followup.

@zoltan
Copy link
Copy Markdown
Collaborator Author

zoltan commented Jun 2, 2026

thank you. let me incorporate everything and update the PR.

@bdice
Copy link
Copy Markdown
Collaborator

bdice commented Jun 3, 2026

@zoltan With apologies, I accidentally pushed to this PR when I meant to push to zoltan#1 (which targeted this branch). In 82e20fe and 7df95a6, I refined the logic and comments to be clearer about how/why ucxx is handled specially. Please feel free to revert anything that you disagree with.

@dan13bauer
Copy link
Copy Markdown
Collaborator

@zoltan Would it also make sense to update scripts/checks/run-clang-tidy.py to exclude ucx-exchange in the same way that cudf is excluded?

Adopt the conventional <LIB>_FOUND pattern per review feedback: derive a
single UCX_FOUND boolean from the find_library/find_path probe and test it
at all three gate sites instead of repeating
`UCX_LIBRARY AND UCX_INCLUDE_DIR`. Pure refactor, no behavior change.

Also correct the stale ucxx pin comment: commit 2e37c84 is the tip of the
release/0.50 branch (VERSION 0.50.00), not a tagged v0.49.00/v0.50.00
release, so annotate it as the branch rather than claiming a release tag.
@zoltan
Copy link
Copy Markdown
Collaborator Author

zoltan commented Jun 3, 2026

@bdice no problem with the push. thanks for the scripts!

@dan13bauer as we have discussed today I think it's better to have a separate PR for that to spark a discussion how we can fix it on the CI so we don't need to exclude it in the future.

@zoltan zoltan changed the title build: Fetch ucxx when system UCX is available build(cudf): bring in ucxx when ucx is present and update all RAPIDS dependencies to latest Jun 3, 2026
@zoltan zoltan changed the title build(cudf): bring in ucxx when ucx is present and update all RAPIDS dependencies to latest build(cudf): Bring in ucxx when ucx is present and update all RAPIDS dependencies to latest Jun 3, 2026
@majetideepak
Copy link
Copy Markdown
Collaborator

@zoltan , @bdice the build is failing. Can you look?

Error: 
 Problem: package cuda-cudart-devel-12-9-12.9.79-1.x86_64 from @System requires cuda-cccl-12-9, but none of the providers can be installed
  - package cccl-13-3-13.3.3.3.1-1.x86_64 from cuda-rhel9-x86_64 obsoletes cuda-cccl-12-9 provided by cuda-cccl-12-9-12.9.27-1.x86_64 from @System
  - package cccl-13-3-13.3.3.3.1-1.x86_64 from cuda-rhel9-x86_64 obsoletes cuda-cccl-12-9 provided by cuda-cccl-12-9-12.9.27-1.x86_64 from cuda-rhel9-x86_64
  - cannot install the best update candidate for package cuda-cudart-devel-12-9-12.9.79-1.x86_64
  - cannot install the best update candidate for package cuda-cccl-12-9-12.9.27-1.x86_64

@zoltan
Copy link
Copy Markdown
Collaborator Author

zoltan commented Jun 4, 2026

we're failing because #17712 is not merged in yet. once it's in, I'll rebase.

@zoltan
Copy link
Copy Markdown
Collaborator Author

zoltan commented Jun 4, 2026

@bdice @czentgr #17637 is a prereq for this PR. ucxx needs CMake 4.0 as a minimum. Bradley, either that is not in-line across RAPIDS (if other RAPIDS component do not need 4.0) or we need this bump anyway before we can update dependency versions?

@bdice
Copy link
Copy Markdown
Collaborator

bdice commented Jun 4, 2026

@zoltan We need to land #17704 (which also has some cuDF version updates needed to fix a bug) and #17712, then I will fix up the versions pinned in this PR. We have to pin to cuDF (and dependencies) from 26.06 until we support CMake 4.0 (#17637), but that shouldn't be a blocker for this PR -- we can pin to 26.06 for now rather than going to 26.08.

This is on my radar and I'll help fix it up once we get the other CI blockers resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants