Skip to content

feat: add libwholegraph wheel#182

Merged
rapids-bot[bot] merged 45 commits intorapidsai:branch-25.06from
gforsyth:wholegraph_wheel
Apr 23, 2025
Merged

feat: add libwholegraph wheel#182
rapids-bot[bot] merged 45 commits intorapidsai:branch-25.06from
gforsyth:wholegraph_wheel

Conversation

@gforsyth
Copy link
Contributor

@gforsyth gforsyth commented Apr 16, 2025

Contributes to rapidsai/build-planning#33

Resolves rapidsai/rapids-wheels-planning#59

This PR proposes packaging libwholegraph as a wheel, which is then re-used by pylibwholegraph, cugraph-dgl, and cugraph-pyg.

It should avoid recompiling RAFT and RMM, as those are now both handled by their respective wheels.

The wholegraph library is now compiled as a shared library.

@copy-pr-bot
Copy link

copy-pr-bot bot commented Apr 16, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@gforsyth gforsyth added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Apr 16, 2025
@gforsyth
Copy link
Contributor Author

/ok to test

@gforsyth
Copy link
Contributor Author

/ok to test

1 similar comment
@gforsyth
Copy link
Contributor Author

/ok to test

@gforsyth
Copy link
Contributor Author

/ok to test

@gforsyth
Copy link
Contributor Author

/ok to test

1 similar comment
@gforsyth
Copy link
Contributor Author

/ok to test

Copy link
Member

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Hope you don't mind me crashing in, but I was interested when I saw @alexbarghi-nv post that this was up. Put up some comments for your consideration.

Leaving a "comment" review so my review doesn't block merging in case you get approvals between now and the next time I'm able to look at this.

@bdice
Copy link
Contributor

bdice commented Apr 22, 2025

The pip devcontainer build is looking for libwholegraph-cu12, but it should just look for the unsuffixed package iirc. I think you need some kind of cuda_suffixed: "true" thing like we do here: https://github.com/rapidsai/cudf/blob/bd588a8949de004685ffbf953d817c003006a3e6/dependencies.yaml#L1020-L1033

Also you might need to update the devcontainers like we did here for cuml: rapidsai/devcontainers#442. Get that PR ready first, and we'll merge it just before we merge this one (to avoid an admin-merge cycle).

@gforsyth
Copy link
Contributor Author

The pip devcontainer build is looking for libwholegraph-cu12, but it should just look for the unsuffixed package iirc.

The pip devcontainer build is looking for libwholegraph-cu12, but it should just look for the unsuffixed package iirc. I think you need some kind of cuda_suffixed: "true" thing like we do here

There is an unsuffixed fallback in the dependencies.yaml -- I think it should be looking for the cu12 variant because it's running pip under cuda 12.8.

I think it's failing because there are no libwholegraph wheels until this gets merged.

@jameslamb
Copy link
Member

I think it's failing because there are no libwholegraph wheels until this gets merged.

That's not it. libwholegraph wheels should never be downloaded in devcontainers builds here. There's code in the devcontainers setup to install dependencies, and it filters out of that anything that it knows it's building directly from source. Here's where that happens: https://github.com/rapidsai/devcontainers/blob/branch-25.06/features/src/rapids-build-utils/opt/rapids-build-utils/bin/make-pip-dependencies.sh#L182-L191

@bdice 's advice in #182 (comment) is the fix... get a libwholegraph entry into the rapids-build-utils manifest in the devcontainers repo, and that should fix the build here.

@gforsyth
Copy link
Contributor Author

devcontainers are currently broken by ucxx -- should that block this from going in?

@bdice
Copy link
Contributor

bdice commented Apr 22, 2025

devcontainers are currently broken by ucxx -- should that block this from going in?

Replied here: rapidsai/devcontainers#494 (comment)

Copy link
Member

@alexbarghi-nv alexbarghi-nv left a comment

Choose a reason for hiding this comment

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

Approved; mostly deferring to the ops team on the finer details of the build process.

@bdice
Copy link
Contributor

bdice commented Apr 23, 2025

All looks fine here with one required follow-up: we're shipping a ~350MB wheel but the conda package is only 18MB. The difference appears to be in NCCL. We need to stop shipping NCCL in this wheel as @divyegala recently did with RAFT and cuVS.

@gforsyth Here's what I propose:

@gforsyth
Copy link
Contributor Author

Yeah, I have a note down for the NCCL devendoring followup

bdice added a commit to rapidsai/devcontainers that referenced this pull request Apr 23, 2025
xref: rapidsai/build-planning#33
xref: rapidsai/cugraph-gnn#182

Add `libwholegraph` wheels to the build manifest.

Currently pointing at my fork in `cugraph-gnn` for testing.

---------

Co-authored-by: Bradley Dice <bdice@bradleydice.com>
@gforsyth
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 2dd02f9 into rapidsai:branch-25.06 Apr 23, 2025
68 checks passed
@gforsyth gforsyth deleted the wholegraph_wheel branch April 23, 2025 15:49
rapids-bot bot pushed a commit that referenced this pull request Jan 20, 2026
Followup to #182, related to #386 (review)

Proposes using `--no-build-isolation` for `libwholegraph` wheel builds. `--no-build-isolation` preserves all of the source file paths, which should lead to better `sccache` hit rate and faster builds.

All of the other `lib{something}` wheels in RAPIDS are built this way, see rapidsai/build-planning#108

Also switches `cugraph-pyg` builds to smaller runners (cpu4). That's a pure-Python package that shouldn't need many resources to build.

Authors:
  - James Lamb (https://github.com/jameslamb)

Approvers:
  - Bradley Dice (https://github.com/bdice)

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

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants