cuproj: depend on librmm, not rmm #1448
Merged
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
jameslamb
added
improvement
github-actions
bot
added
conda
jameslamb
commented
Aug 29, 2024
| requires: | ||
| - cupy>=12.0.0 | ||
| - cuspatial ={{ minor_version }} | ||
| - rmm ={{ minor_version }} |
There was a problem hiding this comment.
cuproj's tests do use cuspatial, but those tests aren't run during conda builds. The only test being run here is import cuproj, which shouldn't need any additional test-specific dependencies (if it did, that might indicate a missing run: dependency!).
jameslamb
changed the title
KyleFromNVIDIA
approved these changes
Aug 29, 2024
|
/merge |
3 tasks
rapids-bot bot
pushed a commit
that referenced
this pull request
Sep 9, 2024
Follow-up to #1448. This proposes some `dependencies.yaml` changes that I noticed while working on adding `libcuspatial` wheels in #1450. I've left inline comments with more details, but in short: * not including any cuspatial projects in the conda environment files checked into this repo (which are intended to be used to set up a development environment) * removing an unnecessary conda-only list of dependencies from pyproject.toml-specific config for `cuproj` * introduction of `depends_on_libcuspatial` and `depends_on_cuproj` lists in `dependencies.yaml`, to reduce duplication Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1451
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.
Description
Contributes to rapidsai/build-planning#33
cuprojdoes not need thermmPython package... it only needs the RMM headers at build time. This proposes the following changes forcuproj:rmmin wheels and conda packagesrmmtolibrmmfor wheels and conda packagestest:environment for conda packagesFor more context on these changes, see rapidsai/build-planning#92.
Notes for Reviewers
Benefits of these changes
Faster conda builds (via dropping unnecessary dependencies).
Cheaper (in terms of bandwidth and disk space) installation of wheels and conda packages (via removing an unnecessary runtime dependency).
Reduces a source of network calls (and therefore CI instability) by removing some CPM downloads of RMM.
Before:
(build link)
After (this PR):
(build link)
Is this required for
libcuspatialwheel packaging?No, it's just a side thing I noticed while working on that. The two are totally independent.
Checklist