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

Use rapids-build-backend #2331

Merged
merged 13 commits into from
May 24, 2024

Conversation

KyleFromNVIDIA
Copy link
Contributor

@KyleFromNVIDIA KyleFromNVIDIA requested review from a team as code owners May 22, 2024 19:41
@KyleFromNVIDIA KyleFromNVIDIA requested a review from jameslamb May 22, 2024 19:41
@KyleFromNVIDIA KyleFromNVIDIA changed the base branch from branch-24.06 to branch-24.08 May 22, 2024 19:42
@KyleFromNVIDIA KyleFromNVIDIA changed the title Rapids build backend Use rapids-build-backend May 22, 2024
@KyleFromNVIDIA KyleFromNVIDIA added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 22, 2024
Copy link
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

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

Approving assuming the upstream RBB issue with forwarding config args is fixed.

@KyleFromNVIDIA
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit b63caf8 into rapidsai:branch-24.08 May 24, 2024
69 checks passed
rapids-bot bot pushed a commit that referenced this pull request Jun 3, 2024
… run, fix wheel dependencies (#2349)

Fixes #2348 

#2331 introduced `rapids-build-backend` (https://github.com/rapidsai/rapids-build-backend) as the build backend for `pylibraft`, `raft-dask`, and `raft-ann-bench`.

That library handles automatically modifying a wheel's dependencies based on the target CUDA version. Unfortunately, we missed a few cases in #2331, and as a result the last few days of nightly `raft-dask` wheels had the following issues:

* depending on `pylibraft`
  - *(does not exist, it's called `pylibraft-cu12`)*
* depending on `ucx-py==0.39.*`
  - *(does not exist, it's called `ucx-py-cu12`)*
* depending on `distributed-ucxx-cu11==0.39.*` instead of `distributed-ucxx-cu11==0.39.*,>=0.0.0a0`
   - *(without that alpha specifier, `pip install --pre` is required to install pre-release versions)*

This wasn't caught in `raft`'s CI, but in downstream CI like `cuml` and `cugraph`, with errors like this:

```text
ERROR: ResolutionImpossible:
  
The conflict is caused by:
    raft-dask-cu12 24.8.0a20 depends on pylibraft==24.8.* and >=0.0.0a0
    raft-dask-cu12 24.8.0a19 depends on pylibraft==24.8.* and >=0.0.0a0
```

([example cugraph build](https://github.com/rapidsai/cugraph/actions/runs/9315062495/job/25656684762?pr=4454#step:7:1811))

This PR:

* fixes those dependency issues
* modifies `raft`'s CI so that similar issues would be caught here in the future, before publishing wheels

## Notes for Reviewers

### What was the root cause of CI missing this, and how does this PR fix it?

The `raft-dask` test CI jobs use this pattern to install the `raft-dask` wheel built earlier in the CI pipeline.

```shell
pip install "raft_dask-cu12[test]>=0.0.0a0" --find-links dist/
```

As described in the `pip` docs ([link](https://pip.pypa.io/en/stable/cli/pip_install/#finding-packages)), `--find-links` just adds a directory to the list of other places `pip` searches for packages. Because the wheel there had unsatisfiable constraints (e.g. `pylibraft==24.8.*` does not exist anywhere), `pip install` silently disregarded that locally-downloaded `raft_dask` wheel and backtracked (i.e. downloaded older and older wheels from https://pypi.anaconda.org/rapidsai-wheels-nightly/simple/) until it found one that wasn't problematic.

This PR ensures that won't happen by telling `pip` to install **exactly that locally-downloaded file**, like this

```shell
pip install "$(echo ./dist/raft_dask_cu12*.whl)[test]"
```

If that file is uninstallable, `pip install` fails and you find out via a CI failure.

### How I tested this

Initially pushed a commit with just the changes to the test script. Saw the `wheel-tests-raft-dask` CI jobs fail in the expected way, instead of silently falling back to an older wheel and passing 🎉 .

```text
ERROR: Could not find a version that satisfies the requirement ucx-py-cu12==0.39.* (from raft-dask-cu12[test]) (from versions: 0.32.0, 0.33.0, 0.34.0, 0.35.0, 0.36.0, 0.37.0, 0.38.0a4, 0.38.0a5, 0.38.0a6, 0.39.0a0)
ERROR: No matching distribution found for ucx-py-cu12==0.39.*
```

([build link](https://github.com/rapidsai/raft/actions/runs/9323598882/job/25668146747?pr=2349))

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

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Dante Gama Dessavre (https://github.com/dantegd)

URL: #2349
rapids-bot bot pushed a commit that referenced this pull request Jun 4, 2024
…onstants (#2344)

Contributes to rapidsai/build-planning#31

Follow-up to #2331

* ensures that `update-version.sh` does not remove alpha specs like `,>=0.0.0a0` in `pyproject.toml` and conda environment files
* consolidates `rapids-build-backend` versions in `dependencies.yaml`
   - *since I was pushing a new commit here anyway, figured I'd take the opportunity to include that simplification recommended in rapidsai/cudf#15245 (comment)
* adds tests that `__git_commit__` and `__version__` constants are present and that `__version__` is populated

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

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Corey J. Nolet (https://github.com/cjnolet)

URL: #2344
rapids-bot bot pushed a commit that referenced this pull request Jun 14, 2024
… followup (#2360)

Contributes to rapidsai/build-planning#31
Contributes to rapidsai/dependency-file-generator#89

Since #2331 was merged, we've made some small adjustments to the approach for `rapids-build-backend`. This catches `raft` up with those changes:

* consolidates version-handling in `ci/` scripts
* uses `--file-key` instead of `--file_key` in `rapids-dependency-file-generator` calls
* reduces duplication of `rapids-build-backend` version constraint in `dependencies.yaml`, and renames some lists there to match the patterns used in other projects (e.g. `rapids_build_skbuild` instead of `build`)

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

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

URL: #2360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci improvement Improvement / enhancement to an existing function non-breaking Non-breaking change python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants