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

WIP: [DO NOT MERGE] introduce libcugraph wheels #4804

Draft
wants to merge 57 commits into
base: branch-25.02
Choose a base branch
from

Conversation

jameslamb
Copy link
Member

@jameslamb jameslamb commented Dec 5, 2024

Replaces #4340, contributes to rapidsai/build-planning#33.

Blocked by rapidsai/raft#2531

Proposes packaging libcugraph as a wheel, which is then re-used by cugraph-cu{11,12} and pylibcugraph-cu{11,12} wheels.

Notes for Reviewers

If you see this note, that means this is not ready for review.

Dependency Flows

---
title: Build & runtime dependencies
---
flowchart TD
    A[libcugraph] --> B[pylibcugraph]
    A --> C[cugraph]
    B --> C
Loading

Size changes (CUDA 12, Python 3.12, x86_64)

wheel num files (before) num files (after) size (before) size (this PR)
libcugraph --- --- --- ---
pylibcugraph 190 --- 900M ---
cugraph 314 --- 901M ---
TOTAL 504 --- 1.8G ---

NOTES: size = compressed, "before" = 2024-12-06 nightlies (5c8f850)

how I calculated those (click me)
docker run \
    --rm \
    -v $(pwd):/opt/work:ro \
    -w /opt/work \
    --network host \
    --env RAPIDS_NIGHTLY_DATE=2024-12-06 \
    --env RAPIDS_NIGHTLY_SHA=5c8f850 \
    --env RAPIDS_PR_NUMBER=4804 \
    --env RAPIDS_PY_CUDA_SUFFIX=cu12 \
    --env RAPIDS_REPOSITORY=rapidsai/cugraph \
    --env WHEEL_DIR_BEFORE=/tmp/wheels-before \
    --env WHEEL_DIR_AFTER=/tmp/wheels-after \
    -it rapidsai/ci-wheel:cuda12.5.1-rockylinux8-py3.12 \
    bash

mkdir -p "${WHEEL_DIR_BEFORE}"
mkdir -p "${WHEEL_DIR_AFTER}"

cpp_projects=(
    libcugraph
)
py_projects=(
    cugraph
    pylibcugraph
)

for project in "${py_projects[@]}"; do
    # before
    RAPIDS_BUILD_TYPE=nightly \
    RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \
    RAPIDS_REF_NAME="branch-25.02" \
    RAPIDS_SHA=${RAPIDS_NIGHTLY_SHA} \
        rapids-download-wheels-from-s3 python "${WHEEL_DIR_BEFORE}"

    # after
    #RAPIDS_BUILD_TYPE=pull-request \
    #RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \
    #RAPIDS_REF_NAME="pull-request/${RAPIDS_PR_NUMBER}" \
    #    rapids-download-wheels-from-s3 python "${WHEEL_DIR_AFTER}"
done

for project in "${cpp_projects[@]}"; do    
    # after
    RAPIDS_BUILD_TYPE=pull-request \
    RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \
    RAPIDS_REF_NAME="pull-request/${RAPIDS_PR_NUMBER}" \
        rapids-download-wheels-from-s3 cpp "${WHEEL_DIR_AFTER}"
done

pip install pydistcheck
pydistcheck \
    --inspect \
    --select 'distro-too-large-compressed' \
    ${WHEEL_DIR_BEFORE}/*.whl

pydistcheck \
    --inspect \
    --select 'distro-too-large-compressed' \
    ${WHEEL_DIR_AFTER}/*.whl

This comment was marked as resolved.

rapids-bot bot pushed a commit that referenced this pull request Dec 20, 2024
In #4804, I've started working on adding `libcugraph` wheels.

This includes a few fixes for things I noticed while doing that:

* removes `wget` from `test_notebook` environment
  - *our CI images already have this system-installed, and removing it helps to remove it from the environment solve for the unified RAPIDS devcontainers*
* `dependencies.yaml` re-organization:
  - breaks `librmm` dependency out into a `depends_on_librmm` to reduce duplication, and for consistency with other RAPIDS dependencies
  - uses `depends_on_pylibwholegraph` group everywhere instead of repeating `pylibwholegraph` in multiple places
  - removes unused YAML anchors
  - alphabetizes lists

## Notes for Reviewers

I'd originally wanted to also add a `librmm` wheel dependency for wheel builds here, but looks like doing that resulted in a lot more `sccache` misses, I guess because of building with build isolation. That change will happen in #4804 .

Authors:
  - James Lamb (https://github.com/jameslamb)
  - Ralph Liu (https://github.com/nv-rliu)

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

URL: #4805
@jameslamb jameslamb changed the title WIP: introduce libcugraph wheels WIP: [DO NOT MERGE] introduce libcugraph wheels Dec 30, 2024
@github-actions github-actions bot added the conda label Dec 30, 2024
rapids-bot bot pushed a commit that referenced this pull request Jan 9, 2025
…anges (#4847)

Proposes some changes to small things I noticed while working on #4804.

* CMake option cleanup:
   - adds `BUILD_PRIMS_BENCH OFF`, removes `BUILD_BENCH OFF` in `get_raft.cmake` (matching changes to RAFT from 23.04: rapidsai/raft#1304)
  - adds `BUILD_BENCHMARKS OFF` in `get_cudf.cmake` ([this is the default](https://github.com/rapidsai/cudf/blob/b81d9e17fbffbb912e0128148f556bf7af41b6ab/cpp/CMakeLists.txt#L51), but better to be explicit)
* consolidates some `.gitignore` rules, adds wheels and conda packages there
* moves responsibility for installing CI artifacts into `ci/test_wheel_{package}.sh` and out of `ci/test_wheel.sh`
* splits up Cython and `scikit-build-core` in `dependencies.yaml`
  - *every Python package here using Cython also uses `scikit-build-core`, but the reverse won't be true as of #4804 ... making that change here is harmless and reduces the size of the diff in #4804 a bit*

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

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

URL: #4847
@github-actions github-actions bot removed the conda label Jan 9, 2025
rapids-bot bot pushed a commit that referenced this pull request Jan 13, 2025
While testing stuff for #4804, I found that `pylibcugraph` has a hard runtime dependency on `cupy` and `numpy`, but isn't declaring them
 
```shell
docker run \
    --rm \
    --gpus 0 \
    -it rapidsai/ci-wheel:latest \
    bash

python -m pip install 'pylibcugraph-cu12==25.2.*,>=0.0.0a0'
python -c "import pylibcugraph"
```

```text
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/pyenv/versions/3.12.7/lib/python3.12/site-packages/pylibcugraph/__init__.py", line 18, in <module>
    from pylibcugraph.graphs import SGGraph, MGGraph
  File "graphs.pyx", line 1, in init pylibcugraph.graphs
  File "utils.pyx", line 20, in init pylibcugraph.utils
ModuleNotFoundError: No module named 'cupy'
```

This declares those dependencies.

It also promotes `cugraph-service-server`'s `numpy` dependency to a hard runtime dependency.

## Notes for Reviewers

### Evidence that `pylibcugraph` *already* has a hard dependency on these libraries

They're used unconditionally here:

https://github.com/rapidsai/cugraph/blob/cddd69ea3f62cabdb3aa2b7b6676e0b74ab4eefc/python/pylibcugraph/pylibcugraph/utils.pyx#L19-L20

But have import guards in other places:

https://github.com/rapidsai/cugraph/blob/cddd69ea3f62cabdb3aa2b7b6676e0b74ab4eefc/python/pylibcugraph/pylibcugraph/sssp.pyx#L127-L139

So this PR doesn't introduce new hard dependencies... it just makes them explicit, to make it easier to install and run `pylibcugraph`.

### How was this not caught in CI?

Import tests aren't run here for conda packages, because conda builds happen on CPU-only nodes.

https://github.com/rapidsai/cugraph/blob/cddd69ea3f62cabdb3aa2b7b6676e0b74ab4eefc/ci/build_python.sh#L27-L30

And `numpy` and `cupy` are probably getting pulled in by some of the wheels' test dependencies, like `cudf`, here:

https://github.com/rapidsai/cugraph/blob/cddd69ea3f62cabdb3aa2b7b6676e0b74ab4eefc/ci/test_wheel.sh#L17

### Should we just make the other unconditional cases conditional with try-catching?

No. Talked with @rlratzel, @ChuckHastings, and @eriknw offline, and we agreed to declare these as hard runtime dependencies (and remove the try-catching in places that had it).

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

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

URL: #4854
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci CMake DO NOT MERGE Hold off on merging; see PR for details python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants