Use CUDA math wheels #2415
Merged
Use CUDA math wheels #2415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KyleFromNVIDIA
added
improvement
robertmaynard
approved these changes
Aug 19, 2024
|
Comparing 32f3703 to this branch with the OldBuild Time: 6m29s NewBuild Time: 5m11s |
bdice
approved these changes
Aug 20, 2024
robertmaynard
approved these changes
Aug 20, 2024
jameslamb
requested changes
Aug 21, 2024
| @@ -124,7 +128,7 @@ requires = [ | |||
| "rmm==24.10.*,>=0.0.0a0", | |||
| ] # This list was generated by `rapids-dependency-file-generator`. To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`. | |||
| dependencies-file = "../../dependencies.yaml" | |||
| matrix-entry = "cuda_suffixed=true" | |||
| matrix-entry = "cuda_suffixed=true;use_cuda_wheels=true" | |||
There was a problem hiding this comment.
When we first talked about this idea, I was thinking usa_cuda_wheels should default to false, so that DLFW / pip devcontainers don't even have to think about it.
And then build scripts here in RAPIDS repos that prepare wheels for distribution would opt in to it by passing use_cuda_wheels=true through config settings.
As I type that, I've come around to the idea that that's not the right balance... one change in DLFW + one change in pip devcontainers is preferable to spreading this complexity around every build_wheel*.sh across RAPIDS.
So I'm good with this, but once we merge a PR with this pattern, please do go put up the corresponding changes in RAPIDS DLFW and devcontainers (devcontainers example: rapidsai/devcontainers#365).
Unused matrix selectors are harmless (I think), so use_cuda_wheels=false could be added unconditionally in those builds even if not all repos have matrices using that.
There was a problem hiding this comment.
Thanks, I added a tasklist with this in it to rapidsai/build-planning#35
There was a problem hiding this comment.
One thing I'd like to point out is that devcontainers generates a requirements.txt file, while this dependency set only has pyproject.toml output, so it won't actually get the wheels anyway. Still, I'll submit a PR just in case that ever changes.
There was a problem hiding this comment.
There was a problem hiding this comment.
devcontainers generates a requirements.txt file, while this dependency set only has pyproject.toml output
I think your conclusion that the devcontainers PR isn't strictly necessary and is just defensive is correct. Everything below is for the benefit of others reading along or finding this from search, and doesn't change this PR.
The claim that this having only output_type: pyproject means we don't need to care about it for pip devcontainers isn't true by itself.
The runtime dependencies in the wheel metadata don't strictly have to be correct because in pip devcontainers, the package will be built with pip install --no-deps --no-build-isolation.
That --no-deps does mean that the runtime dependencies listed in the wheel's metadata don't have to be correct.
So it's output_type: pyproject and the fact that these CUDA wheels are only runtime dependencies that together make this change not disruptive for RAPIDS devcontainers.
HOWEVER... if the project metadata lists any build dependencies that don't already exist in the environment where that pip install runs, that build will fail.
So any list in dependencies.yaml which had only output_type: pyproject which was used to populate wheel build requirements could affect pip devcontainers, because those requirements would need to all be met by the pre-created build environment.
jameslamb
approved these changes
Aug 21, 2024
|
/merge |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use CUDA math wheels to reduce wheel size by not statically linking CUDA math libraries.
Contributes to rapidsai/build-planning#35