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

Remove CUDA 12.2 conda builds. #233

Merged
merged 2 commits into from
Jul 25, 2024
Merged

Remove CUDA 12.2 conda builds. #233

merged 2 commits into from
Jul 25, 2024

Conversation

bdice
Copy link
Contributor

@bdice bdice commented Jul 25, 2024

This PR removes CUDA 12.2 conda builds. There’s no need for those jobs because conda packages are only specific to the CUDA major version. If I remember the artifact naming scheme correctly, we run the risk of testing the wrong built artifacts if two minor versions from the same major family are used to build packages.

For testing, our intention is to build with 12.5 and use those packages for all CUDA 12 tests (12.0, 12.2, 12.5, etc.).

@bdice bdice requested a review from a team as a code owner July 25, 2024 06:22
@bdice bdice requested review from msarahan and removed request for a team July 25, 2024 06:22
@jakirkham
Copy link
Member

Thanks Bradley! 🙏

Would recommend reading this comment from James: #229 (review)

@jakirkham jakirkham requested review from jameslamb and KyleFromNVIDIA and removed request for msarahan July 25, 2024 06:46
@bdice
Copy link
Contributor Author

bdice commented Jul 25, 2024

We can retain testing coverage but not build coverage. We only need artifacts for one minor version due to CUDA Enhanced Compatibility. However, building multiple minor versions gives artifacts that can clash.

I am not on VPN at the moment to verify but I am pretty sure this is the case. Artifacts are named like ./linux-64/libcudf-24.08.00a390-cuda12_240725_g4aefcc7b29_390.tar.bz2 with no indication of the minor version. The choice between fetching CUDA 12.2 builds and 12.5 builds at test time is arbitrary, iirc, so we should provide only one build.

Keeping only one build per major CUDA version is the same way we handled 12.0 to 12.2 migration, and all past CUDA 11 migrations, iirc.

I’ll verify this further tomorrow.

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.

You are 100% right! Here's what the artifact names look like for cuml, for example (only cuda12, no minor version):

image

https://downloads.rapids.ai/nightly/cuml/2024-07-25/6357c64/

Sorry, I'd asked that these build jobs be added in. Totally forgot about the artifact-naming thing and misunderstood the relationship between the versions we build against and the versions we test against.

@jameslamb
Copy link
Member

/merge

@bdice bdice merged commit 115cba6 into branch-24.08 Jul 25, 2024
@jakirkham jakirkham deleted the remove-12.2-builds branch July 25, 2024 21:53
@jakirkham
Copy link
Member

Thanks all! 🙏

Agree this is an important fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants