Use rapids-build-backend #5804
Use rapids-build-backend #5804
Conversation
KyleFromNVIDIA
added
5 - DO NOT MERGE
|
@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. |
|
Could you please use mentions in the PR description to link these PRs to the main issue they're contributing to? e.g. like this:
Think that'd help library teams to trace back to the context for these changes, and it makes it easier to go from the main issue to all of the changes across RAPIDS that it led to. I've found those types of links really useful in reconstructing the context for why something was done in the past. |
| specific: | ||
| - output_types: [conda, requirements, pyproject] | ||
| matrices: | ||
| - matrix: | ||
| cuda: "12.*" | ||
| packages: | ||
| - cuda-python>=12.0,<13.0a0 | ||
| - matrix: # All CUDA 11 versions | ||
| - matrix: | ||
| cuda: "11.*" |
There was a problem hiding this comment.
Oh my gosh, thanks for fixing this. I would've totally overlooked that until a later PR. Be on the lookout for this type of cleanup across RAPIDS.
Related: can we remove the empty matrix below? In my understanding all builds should be providing matrix to DFG now, so there should never be a fallback matrix required.
There was a problem hiding this comment.
The empty matrix is needed for the pre-commit hook to run properly, since there is no default matrix for pyproject.toml.
python/pyproject.toml
Outdated
| "cupy-cuda11x>=12.0.0", | ||
| "dask-cuda==24.6.*", | ||
| "dask-cudf==24.6.*", | ||
| "dask-cuda==24.6.*,>=0.0.0a0", |
There was a problem hiding this comment.
I've forgotten how this is supposed to work -- didn't we need to preserve the suffixless packages like cudf==24.6.* in this file's default contents for the purpose of devcontainers?
There was a problem hiding this comment.
@trxcllnt can you attest to this?
There was a problem hiding this comment.
devcontainers don't care about the contents of pyproject.toml at all. What they care about is the contents of dependencies.yaml, specifically the requirements outputs (for pip devcontainers; conda devcontainers use the conda outputs). So what we need to maintain for devcontainers is a fallback for matrix entries of suffixed packages where the fallback is not suffixed.
That being said, I'm not sure I understand the rationale for the current contents of pyproject.toml. Why are so many dependencies removed from the default committed version of these lists? I feel like these lists should either contain all unsuffixed versions of every dependency to begin with, or these lists should be entirely empty and be filled in by RBB. Why this half-measure?
There was a problem hiding this comment.
The other relevant piece to be aware of for devcontainers is that in devcontainers we may need to control whether a package itself has a suffixed name or not. That's what rapidsai/rapids-build-backend#17 (comment) is about. @trxcllnt can comment more on that use case.
There was a problem hiding this comment.
Why this half-measure?
The ones that are currently in there are common between every matrix configuration. I don't think it makes sense to have unsuffixed dependencies in there, since they will never actually be used in practice.
There was a problem hiding this comment.
That's fair. I had originally considered that it might be more comprehensible to anyone actually inspecting the pyproject.toml file to see real-ish dependency lists, but maybe that doesn't matter.
There was a problem hiding this comment.
I concur with @vyasr’s earlier comment here. Having this list be “real-ish” is actually very helpful when explaining things to users/devs, even if it’s never used directly. I don’t like the awkward halfway-correct state.
There was a problem hiding this comment.
OK, I've added back the fake dependencies.
There was a problem hiding this comment.
Just adding the summary from our offline conversations last week, for those finding this thread in the future.
It's important (for RAPIDS devcontainers and DLFW builds) that the fallback matrices in dependencies.yaml have non-CUDA-specific dependencies. e.g. like this:
- output_types: [requirements, pyproject]
matrices:
- matrix: {cuda: "12.*"}
packages:
- pylibraft-cu12==24.6.*,>=0.0.0a0
- rmm-cu12==24.6.*,>=0.0.0a0
- matrix: {cuda: "11.*"}
packages:
- pylibraft-cu11==24.6.*,>=0.0.0a0
- rmm-cu11==24.6.*,>=0.0.0a0
- matrix:
packages:
- pylibraft==24.6.*,>=0.0.0a0
- rmm==24.6.*,>=0.0.0a0As a side effect of how the rapids-dependency-file-generator pre-commit hook works across RAPIDS repos, that being true would also mean non-CUDA-specific dependencies in pyproject.toml in source control.
python/pyproject.toml
Outdated
| "cupy-cuda11x>=12.0.0", | ||
| "dask-cuda==24.6.*", | ||
| "dask-cudf==24.6.*", | ||
| "dask-cuda==24.6.*,>=0.0.0a0", |
There was a problem hiding this comment.
devcontainers don't care about the contents of pyproject.toml at all. What they care about is the contents of dependencies.yaml, specifically the requirements outputs (for pip devcontainers; conda devcontainers use the conda outputs). So what we need to maintain for devcontainers is a fallback for matrix entries of suffixed packages where the fallback is not suffixed.
That being said, I'm not sure I understand the rationale for the current contents of pyproject.toml. Why are so many dependencies removed from the default committed version of these lists? I feel like these lists should either contain all unsuffixed versions of every dependency to begin with, or these lists should be entirely empty and be filled in by RBB. Why this half-measure?
KyleFromNVIDIA
removed
the
5 - DO NOT MERGE
Contributes to rapidsai/build-planning#31.