refactor(rattler): remove cuda 11 branching #19039
Conversation
There was a problem hiding this comment.
Thanks Gil! 🙏
Guessing this was done with some kind of replacement script or similar, is that right? Having pure YAML definitely makes this easier to do
Think it did some things well (like requirements)
However with ignore_run_exports* it appears to have dropped some packages (especially if there was cuda_major != "11")
Also it add the compilers and C stdlib to this section, which doesn't make sense to me (though please let me know if I'm missing something). Wondering if that might also be a cause of the CI failures
In any event please take a look at the comments below and let me know what you think
Once we converge on an approach here, we should propagate it to the other PRs. Will stick to reviewing this one so we have one point of discussion until then
conda/recipes/cudf/recipe.yaml
Outdated
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
Also don't think we want these here, but please correct me if I'm missing something
| - ${{ compiler("c") }} | |
| - ${{ compiler("cxx") }} | |
| - ${{ compiler("cuda") }} | |
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
I added those intentionally -- rattler didn't support ignoring the compiler packages originally, but it does now so I took the opportunity to add those in.
There was a problem hiding this comment.
Just following up here -- this was a bit of a crossed wire in my head. There are certain outputs in certain recipes that have unnecessary run exports inherited from the compilers in the cache-build, but most outputs need those exports. I'm going through more deliberately now and fixing those up.
conda/recipes/cudf_kafka/recipe.yaml
Outdated
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
| - ${{ compiler("c") }} | |
| - ${{ compiler("cxx") }} | |
| - ${{ compiler("cuda") }} | |
| - ${{ stdlib("c") }} |
conda/recipes/libcudf/recipe.yaml
Outdated
| from_package: | ||
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
| from_package: | |
| - ${{ compiler("c") }} | |
| - ${{ compiler("cxx") }} | |
| - ${{ compiler("cuda") }} | |
| - ${{ stdlib("c") }} |
conda/recipes/libcudf/recipe.yaml
Outdated
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
| - ${{ compiler("c") }} | |
| - ${{ compiler("cxx") }} | |
| - ${{ compiler("cuda") }} | |
| - ${{ stdlib("c") }} |
conda/recipes/libcudf/recipe.yaml
Outdated
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
| - ${{ compiler("c") }} | |
| - ${{ compiler("cxx") }} | |
| - ${{ compiler("cuda") }} | |
| - ${{ stdlib("c") }} |
conda/recipes/pylibcudf/recipe.yaml
Outdated
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
| - ${{ compiler("c") }} | |
| - ${{ compiler("cxx") }} | |
| - ${{ compiler("cuda") }} | |
| - ${{ stdlib("c") }} |
|
Should add am enjoying watching all these CUDA 11 bit drop from our recipes. This will make them much more pleasant to maintain 🤩 |
7db8b69 to
c4ddc10
Compare
conda/recipes/libcudf/recipe.yaml
Outdated
| ignore_run_exports: | ||
| from_package: | ||
| - ${{ compiler("cuda") }} | ||
| by_name: |
There was a problem hiding this comment.
Don't think we want to ignore_run_exports from the ${{ compiler("cuda") }} package
This is a bit of a heavy hammer at this point. For example ${{ compiler("cuda") }} exports arm-variant, which differentiates Tegra and SBSA builds
So instead of this would recommend we simply ignore what we need to from ${{ compiler("cuda") }}. Namely ignoring cuda-version like so
| ignore_run_exports: | |
| from_package: | |
| - ${{ compiler("cuda") }} | |
| by_name: | |
| ignore_run_exports: | |
| by_name: | |
| - cuda-version |
This happens a few other places. Would recommend the same change for those as well
conda/recipes/libcudf/recipe.yaml
Outdated
| requirements: | ||
| build: | ||
| - ${{ compiler("c") }} | ||
| - ${{ compiler("cuda") }} | ||
| - ${{ compiler("cxx") }} | ||
| - ${{ stdlib("c") }} |
There was a problem hiding this comment.
Am curious why the other compiler packages are dropped here? Also why is only ${{ compiler("cuda") }} kept?
There was a problem hiding this comment.
Just for future context -- my initial thought was that we could ignore all the compiler exports. That is incorrect. Then I thought we could at least ignore the cuda compiler exports, since the version pinning is handled by cuda-version, but there are recent changes to account for card variants so we want to keep the cuda compiler exports around, too.
|
There is a Spark test failure. AIUI this was fixed in PR: NVIDIA/spark-rapids-jni#3375 Think we can just try restarting after CI completes |
|
Spark failures are not required to merge, and will be fixed separately. I'll go ahead and merge this. |
|
/merge |
`cudf`'s dependencies on `cubinlinker` and `ptxcompiler` were removed in #19139 and #19039, as part of the effort to drop CUDA 11 support across RAPIDS (rapidsai/build-planning#184). This proposes removing a few more uses of those libraries: * code comments in `dependencies.yaml` * CUDA-major-version-dependent code in `_setup_numba()` Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #19177
Description
Removes all the CUDA 11 branching from the rattler-build recipe.
xref rapidsai/build-planning#184
Checklist