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 #5804

Merged
merged 62 commits into from
Jun 4, 2024

Conversation

KyleFromNVIDIA
Copy link
Contributor

@KyleFromNVIDIA KyleFromNVIDIA commented Mar 14, 2024

Contributes to rapidsai/build-planning#31.

@KyleFromNVIDIA KyleFromNVIDIA added 5 - DO NOT MERGE Hold off on merging; see PR for details improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Mar 14, 2024
@KyleFromNVIDIA KyleFromNVIDIA requested review from a team as code owners March 14, 2024 18:01
@github-actions github-actions bot added Cython / Python Cython or Python issue ci labels Mar 14, 2024
@KyleFromNVIDIA KyleFromNVIDIA requested a review from a team as a code owner March 14, 2024 20:16
@github-actions github-actions bot added the conda conda issue label Mar 14, 2024
@github-actions github-actions bot removed the conda conda issue label Mar 14, 2024
@github-actions github-actions bot added the conda conda issue label Mar 15, 2024
@KyleFromNVIDIA KyleFromNVIDIA changed the base branch from branch-24.04 to branch-24.06 March 19, 2024 15:58
@bdice
Copy link
Contributor

bdice commented Mar 20, 2024

@KyleFromNVIDIA I'm going to merge the upstream into this PR to get a fresh CI run. This is the only open PR targeting 24.06 and it's a good candidate to look at. I want to investigate the latest CI failures, which @rjzamora indicated may be related to dask-expr.

I'll compare its failures to those on #5815.

build.sh Outdated Show resolved Hide resolved
@jameslamb jameslamb self-requested a review May 29, 2024 19:40
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.

This looks good to me, and I think it's ready to merge. Those failing test jobs are optional, and the failures are unrelated to this PR's changes.

@jameslamb jameslamb self-requested a review May 29, 2024 21:56
python/cuml/_version.py Outdated Show resolved Hide resolved
@jameslamb
Copy link
Member

Those 3 failing cudf-pandas-integration test jobs are optional and the failures are not a result of this PR's changes. I'm confident in that because I see the exact same failures on other unrelated PRs, like #5905.

  File "/opt/conda/envs/test/lib/python3.9/site-packages/cuml/preprocessing/LabelEncoder.py", line 121, in LabelEncoder
  Failed example:
      print(str_label)
  Exception raised:
      Traceback (most recent call last):
        File "/opt/conda/envs/test/lib/python3.9/doctest.py", line 1334, in __run
          exec(compile(example.source, filename, "single",
        File "<doctest LabelEncoder[17]>", line 1, in <module>
          print(str_label)
      NameError: name 'str_label' is not defined

(build link)

This PR looks like it has all the changes we want for the rapids-build-backend rollout, so I'm merging it.

@jameslamb
Copy link
Member

/merge

@jameslamb
Copy link
Member

jameslamb commented May 31, 2024

Oh interesting, I see this now

Waiting on code owner review from rapidsai/cuml-cmake-codeowners and/or rapidsai/cuml-python-codeowners.

Even though I don't see those groups in the list of reviewers.

image

I guess the groups were dropped because @bdice and @vyasr are a part of them and left earlier reviews "on behalf of" those groups.

Copy link
Contributor

@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.

Just a few minor alignment questions -- we'd like the build scripts to look as identical as possible across RAPIDS.

version=$(rapids-generate-version)
git_commit=$(git rev-parse HEAD)
export RAPIDS_PACKAGE_VERSION=${version}
Copy link
Contributor

Choose a reason for hiding this comment

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

We changed up how this works for some other packages, to something like rapids-generate-version > ./VERSION. Does this need a similar change?

e.g. https://github.com/rapidsai/cudf/blob/branch-24.08/ci/build_python.sh

Copy link
Member

Choose a reason for hiding this comment

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

Yes I think so, you're right. I'll change that here.

Copy link
Member

Choose a reason for hiding this comment

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

I just pushed 29911b6 doing that.

Made a similar change in build_cpp.sh as well (which doesn't need the version to be written to the VERSION file).

ci/build_wheel.sh Outdated Show resolved Hide resolved
dependencies.yaml Outdated Show resolved Hide resolved
@jameslamb
Copy link
Member

jameslamb commented May 31, 2024

The devcontainer pip job is failing like this:

ERROR: Cannot install cuml-cu11 because these package versions have conflicting dependencies.

The conflict is caused by:
    raft-dask-cu11 24.8.0a20 depends on distributed-ucxx-cu11==0.39.*
    raft-dask-cu11 24.8.0a19 depends on distributed-ucxx-cu11==0.39.*

To fix this you could try to:
1. loosen the range of package versions you've specified
2. remove package versions to allow pip attempt to solve the dependency conflict

(build link)

I believe the root cause is bugs from rapidsai/raft#2331, not caught in CI because of rapidsai/raft#2348, which should be fixed by some combination of rapidsai/raft#2349 and rapidsai/raft#2347.

@jameslamb
Copy link
Member

The only failing tests here are the optional pandas ones, and the failures look unrelated to the changes on this PR (#5804 (comment)). I also double-checked that there's no use of --find-links here (thinking about rapidsai/raft#2348).

I'm confident this can be merged.

@jameslamb
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit 225c5aa into rapidsai:branch-24.08 Jun 4, 2024
60 of 62 checks passed
rapids-bot bot pushed a commit that referenced this pull request Jun 14, 2024
… followup (#5928)

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

#5804 was one of the earlier `rapids-build-backend` PRs merged across RAPIDS. Since it was merged, we've made some small adjustments to the approach for `rapids-build-backend`. This catches `cuml` up with those changes:

* removes unused constants in `ci/build*` scripts
* uses `--file-key` instead of `--file_key` in `rapids-dependency-file-generator` calls
* uses `--prepend-channel` instead of `--prepend-channels` in `rapids-dependency-file-generator` calls
* ensures `ci/update-version.sh` preserves alpha specs

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

Approvers:
  - Ray Douglass (https://github.com/raydouglass)

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

Successfully merging this pull request may close these issues.

5 participants