[RELEASE] cuspatial v24.10 #1466
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
…-24.10-merge-24.08
Forward-merge branch-24.08 into branch-24.10
Forward-merge branch-24.08 into branch-24.10
Forward-merge branch-24.08 into branch-24.10
Forward-merge branch-24.08 into branch-24.10
closes #1427 The API of `cudf.DataFrame.insert` will change in 24.10 to better align with pandas, so adjusting the usage here xref rapidsai/cudf#16402 Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Mark Harris (https://github.com/harrism) URL: #1428
…orkflow (#1429) closes #1426 It appears there was a change in 24.08 that broke a notebook demonstrating a `merge` on two geometry columns. It seems like the merge result tries to reconstruct a `GeoDataFrame` from a `dict[Any, GeoSeries | Series]` but the `Series.index` alignment requires the types to be recognized cudf types (not `"geometry"`) I don't think this alignment is entirely necessary though since it goes through the `_split_out_geometry_columns`/`_recombine_columns` methods which appears to be used on operations that maintain row ordering so index alignment isn't required. This PR instead passes a `dict[Any, GeoColumn | Column]` to `cudf.DataFrame._from_data` given that this row ordering is preserved. (This PR also includes the fix for #1427) Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Mark Harris (https://github.com/harrism) URL: #1429
…#1434) closes #1433 Mirroring the upstream cudf changes in rapidsai/cudf#16454, `cudf.Series` disallows accepting a `ColumnBase` in favor of the `cudf.Series._from_column` constructor. This PR does the same for `GeoSeries` as well as addresses the breakages due to the upstream cudf change. Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Mark Harris (https://github.com/harrism) - Bradley Dice (https://github.com/bdice) URL: #1434
This PR updates pre-commit hooks to the latest versions that are supported without causing style check errors. Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) - Bradley Dice (https://github.com/bdice) Approvers: - James Lamb (https://github.com/jameslamb) URL: #1435
A few small tweaks to `update-version.sh` for alignment across RAPIDS. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Mark Harris (https://github.com/harrism) - James Lamb (https://github.com/jameslamb) URL: #1432
…rsion (#1430) Contributes to rapidsai/build-planning#58. `scikit-build-core==0.10.0` was released today (https://github.com/scikit-build/scikit-build-core/releases/tag/v0.10.0), and wheel-building configurations across RAPIDS are incompatible with it. This proposes upgrading to that version and fixing configuration here in a way that: * is compatible with that new `scikit-build-core` version * takes advantage of the forward-compatibility mechanism (`minimum-version`) that `scikit-build-core` provides, to reduce the risk of needing to do this again in the future Authors: - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - https://github.com/jakirkham URL: #1430
…ty (#1438) closes #1437 When `GeoDataFrame._split_out_geometry_columns` is called, there may be no geometry columns to split out into 2 DataFrames; therefore, once `GeoDataFrame._recombine_columns`, it assumed the 2 DataFrames were always non-empty. Added a check to bypass this validation if either was empty. I ran the `ZipCodes_Stops_PiP_cuSpatial` notebook locally and the (non-commented) cells no longer raise an exception. Authors: - Matthew Roeschke (https://github.com/mroeschke) - James Lamb (https://github.com/jameslamb) Approvers: - Mark Harris (https://github.com/harrism) URL: #1438
The notebook-testing CI job in this repo does not actually cause a loud CI failure if any errors are detected in notebooks. That's because of a `bash` mistake I made in #1407... that PR moved notebook-checking into a function, but didn't add a `set -E` to be sure errors from inside that function were appropriately trapped. This PR fixes that: * ensures that notebook failures actually cause CI failures * fixes 2 typos in `nyc_taxi_years_correlation.ipynb` code - *(not caught in #1422 because of this CI script bug)* Context: #1422 (review) ## Notes for Reviewers ### How I tested this Originally did not skip any notebooks. Saw the failures in one of the notebooks cause an actual merge-blocking CI failure. Build link: https://github.com/rapidsai/cuspatial/actions/runs/10199784404/job/28219162698?pr=1424 # Authors: - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) Approvers: - Ray Douglass (https://github.com/raydouglass) - Mark Harris (https://github.com/harrism) - https://github.com/jakirkham URL: #1424
With rapidsai/cudf#16549, `cudf.Index` no longer accepts `Column` objects. The only usage I found was that `GeoColumn` accepted `shuffle_order=cudf.Index(column)`, but `shuffle_order` appears unused (anymore?) in `GeoColumn`, so I went ahead and removed this from the constructor Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Vyas Ramasubramani (https://github.com/vyasr) URL: #1439
This is necessary for compatibility with cudf after rapidsai/cudf#16299. Authors: - Vyas Ramasubramani (https://github.com/vyasr) Approvers: - Mark Harris (https://github.com/harrism) - Bradley Dice (https://github.com/bdice) URL: #1440
Mark all cuspatial CUDA kernels with internal linkage to avoid runtime symbol collisions. Also updated the CI tests to also verify that we add no new CUDA kernel with external linkage. Authors: - Robert Maynard (https://github.com/robertmaynard) - Bradley Dice (https://github.com/bdice) Approvers: - Mark Harris (https://github.com/harrism) - Bradley Dice (https://github.com/bdice) URL: #1436
Post rapidsai/cudf#16465, the `data` argument to `ListColumn` is a required argument (as `None`) Authors: - Matthew Roeschke (https://github.com/mroeschke) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1442
Contributes to rapidsai/build-planning#88 Finishes the work of dropping Python 3.9 support. This project stopped building / testing against Python 3.9 as of rapidsai/shared-workflows#235. This PR updates configuration and docs to reflect that. ## Notes for Reviewers ### How I tested this Checked that there were no remaining uses like this: ```shell git grep -E '3\.9' git grep '39' git grep 'py39' ``` And similar for variations on Python 3.8 (to catch things that were missed the last time this was done). Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Mark Harris (https://github.com/harrism) - https://github.com/jakirkham URL: #1443
Contributes to rapidsai/build-planning#33. Proposes the following for `cuspatial` wheels: * add build and runtime dependencies on `libcudf` wheels * stop vendoring copies of `libcudf.so`, `libnvcomp.so`, `libnvcomp_bitcomp.so`, and `libnvcomp_gdeflate.so` - *(load `libcudf.so` dynamically at runtime instead)* And other related changes for development/CI: * combine all `pip install` calls into 1 in wheel-testing scripts - *like rapidsai/cudf#16575 - *to improve the chance that packaging issues are discovered in CI* * `dependencies.yaml` changes: - more use of YAML anchors = less duplication - use dedicated `depends_on_librmm` and `depends_on_libcudf` groups * explicitly pass a package type to `gha-tools` wheel uploading/downloading scripts ## Notes for Reviewers ### Benefits of these changes Unblocks CI in this repo (ref: #1444 (comment), #1441 (comment)). Reduces wheel sizes for `cuspatial` wheels by about 125MB 😁 | wheel | size (before) | size (this PR) | |:-----------:|-------------:|---------------:| | `cuspatial` | 146.0M | 21M | | `cuproj ` | 0.9M | 0.9M | |**TOTAL** | **146.9M** | **21.9M** | *NOTES: size = compressed, "before" = 2024-08-21 nightlies (c60bd4d), CUDA = 12, Python = 3.11* <details><summary>how I calculated those (click me)</summary> ```shell # note: 2024-08-21 because that was the most recent date with # successfully-built cuspatial nightlies # docker run \ --rm \ -v $(pwd):/opt/work:ro \ -w /opt/work \ --network host \ --env RAPIDS_NIGHTLY_DATE=2024-08-21 \ --env RAPIDS_NIGHTLY_SHA=c60bd4d \ --env RAPIDS_PR_NUMBER=1447 \ --env RAPIDS_PY_CUDA_SUFFIX=cu12 \ --env RAPIDS_REPOSITORY=rapidsai/cuspatial \ --env WHEEL_DIR_BEFORE=/tmp/wheels-before \ --env WHEEL_DIR_AFTER=/tmp/wheels-after \ -it rapidsai/ci-wheel:cuda12.5.1-rockylinux8-py3.11 \ bash mkdir -p "${WHEEL_DIR_BEFORE}" mkdir -p "${WHEEL_DIR_AFTER}" py_projects=( cuspatial cuproj ) for project in "${py_projects[@]}"; do # before RAPIDS_BUILD_TYPE=nightly \ RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \ RAPIDS_REF_NAME="branch-24.10" \ RAPIDS_SHA=${RAPIDS_NIGHTLY_SHA} \ rapids-download-wheels-from-s3 python "${WHEEL_DIR_BEFORE}" # after RAPIDS_BUILD_TYPE=pull-request \ RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \ RAPIDS_REF_NAME="pull-request/${RAPIDS_PR_NUMBER}" \ rapids-download-wheels-from-s3 python "${WHEEL_DIR_AFTER}" done du -sh ${WHEEL_DIR_BEFORE}/* du -sh ${WHEEL_DIR_BEFORE} du -sh ${WHEEL_DIR_AFTER}/* du -sh ${WHEEL_DIR_AFTER} ``` </details> Reduces the amount of additional work required to start shipping `libcuspatial` wheels. ### Background This is part of ongoing work towards packaging `libcuspatial` as a wheel. relevant prior work: * packaging `libcudf` wheels: rapidsai/cudf#15483 * consolidating `pip install` calls in CI scripts for `cudf`: rapidsai/cudf#16575 * `cudf` dropping its Arrow library dependency: rapidsai/cudf#16640 ### How I tested this Confirmed in local builds and CI logs that `cudf` is being *found*, not *built*, in `cuspatial` builds. ```text -- CPM: Using local package [email protected] ``` ([build link](https://github.com/rapidsai/cuspatial/actions/runs/10602971716/job/29386288614?pr=1447#step:9:23472)) Built `cuspatial` wheels locally and ran all the unit tests, without issue. # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) - Vyas Ramasubramani (https://github.com/vyasr) - Matthew Roeschke (https://github.com/mroeschke) URL: #1447
This PR updates rapidsai/pre-commit-hooks to the version 0.4.0. Authors: - Kyle Edwards (https://github.com/KyleFromNVIDIA) - James Lamb (https://github.com/jameslamb) Approvers: - James Lamb (https://github.com/jameslamb) URL: #1445
Contributes to rapidsai/build-planning#33 `cuproj` does not need the `rmm` Python package... it only needs the RMM headers at build time. This proposes the following changes for `cuproj`: * dropping the runtime requirement on `rmm` in wheels and conda packages * switching the build requirement from `rmm` to `librmm` for wheels and conda packages * removing unnecessary imports in the `test:` environment for conda packages For 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: ```text -- CPM: Adding package [email protected] (branch-24.10) ``` ([build link](https://github.com/rapidsai/cuspatial/actions/runs/10618529204/job/29434041322#step:9:16754)) After (this PR): ```text -- CPM: Using local package [email protected] ``` ([build link](https://github.com/rapidsai/cuspatial/actions/runs/10619138604/job/29436119470?pr=1448#step:9:11256)) ### Is this required for `libcuspatial` wheel packaging? No, it's just a side thing I noticed while working on that. The two are totally independent. # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #1448
Proposes removing the `cuproj` conda package's pin on `proj`, so that it can build using the same version that `conda-forge` pins to in https://github.com/conda-forge/conda-forge-pinning-feedstock. For context, see: * why the `proj==9.3.0` pin was originally added: #1307 * `conda-forge` completied migrations: - `9.3.1`: conda-forge/conda-forge-pinning-feedstock#5401 - `9.4.`: conda-forge/conda-forge-pinning-feedstock#6240 Authors: - James Lamb (https://github.com/jameslamb) Approvers: - https://github.com/jakirkham URL: #1449
RAPIDS libraries recently started running nightly wheel tests on Rocky Linux 8: https://github.com/rapidsai/shared-workflows/pull/236/files#r1725947245 That distribution's system package manager is `yum`, not `apt`, and as a result `cuspatial`'s nightly runs are failing like this: ```text ci/test_wheel_cuspatial.sh: line 10: apt: command not found Error: Process completed with exit code 127. ``` ([build link](https://github.com/rapidsai/cuspatial/actions/runs/10678284262/job/29595047179)) This fixes that. ## Notes for Reviewers ### How I tested this ```shell docker run \ --rm \ --gpus 1 \ -v $(pwd):/opt/work \ -w /opt/work \ -it rapidsai/citestwheel:cuda12.5.1-rockylinux8-py3.11 \ bash yum update -y yum config-manager --set-enabled powertools yum update -y yum install -y gdal-devel python -m pip install \ --no-binary fiona \ 'cuproj-cu12[test]==24.10.*,>=0.0.0a0' \ 'cuspatial-cu12[test]==24.10.*,>=0.0.0a0' \ 'fiona>=1.8.19,<1.9' pushd python/cuproj/cuproj python -m pytest \ --cache-clear \ --numprocesses=8 \ --dist=worksteal \ tests popd pushd python/cuspatial/cuspatial python -m pytest \ --cache-clear \ --numprocesses=8 \ --dist=worksteal \ tests popd ``` # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1452
Contributes to rapidsai/build-planning#33 Adds `libcuspatial` wheels, and switches `cuspatial` wheels to using them. ## Notes for Reviewers ### Benefits of these changes Faster CI runs and smaller total footprint on package repositories (because now `libcuspatial` no longer needs to be compiled once per Python version). Smaller `cuspatial` wheels. | whee. l | size (before) | size (this PR) | |:-------------:|-------------:|----------------:| | `libcuspatial` | --- | 17.0M | | `cuspatial`. | 21.0M | 4.1M | | `cuproj ` | 0.9M | 0.9M | |**TOTAL** | **21.9M** | **22.0M** | *NOTES: size = compressed, "before" = 2024-09-02 nightlies (1544e7b), CUDA = 12, Python = 3.11* <details><summary>how I calculated those (click me)</summary> ```shell docker run \ --rm \ -v $(pwd):/opt/work:ro \ -w /opt/work \ --network host \ --env RAPIDS_NIGHTLY_DATE=2024-09-02 \ --env RAPIDS_NIGHTLY_SHA=1544e7b \ --env RAPIDS_PR_NUMBER=1450 \ --env RAPIDS_PY_CUDA_SUFFIX=cu12 \ --env RAPIDS_REPOSITORY=rapidsai/cuspatial \ --env WHEEL_DIR_BEFORE=/tmp/wheels-before \ --env WHEEL_DIR_AFTER=/tmp/wheels-after \ -it rapidsai/ci-wheel:cuda12.5.1-rockylinux8-py3.11 \ bash mkdir -p "${WHEEL_DIR_BEFORE}" mkdir -p "${WHEEL_DIR_AFTER}" py_projects=( cuspatial cuproj ) for project in "${py_projects[@]}"; do # before RAPIDS_BUILD_TYPE=nightly \ RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \ RAPIDS_REF_NAME="branch-24.10" \ RAPIDS_SHA=${RAPIDS_NIGHTLY_SHA} \ rapids-download-wheels-from-s3 python "${WHEEL_DIR_BEFORE}" # after RAPIDS_BUILD_TYPE=pull-request \ RAPIDS_PY_WHEEL_NAME="${project}_${RAPIDS_PY_CUDA_SUFFIX}" \ RAPIDS_REF_NAME="pull-request/${RAPIDS_PR_NUMBER}" \ rapids-download-wheels-from-s3 python "${WHEEL_DIR_AFTER}" done # after RAPIDS_BUILD_TYPE=pull-request \ RAPIDS_PY_WHEEL_NAME="libcuspatial_${RAPIDS_PY_CUDA_SUFFIX}" \ RAPIDS_REF_NAME="pull-request/${RAPIDS_PR_NUMBER}" \ rapids-download-wheels-from-s3 cpp "${WHEEL_DIR_AFTER}" du -sh ${WHEEL_DIR_BEFORE}/* du -sh ${WHEEL_DIR_BEFORE} du -sh ${WHEEL_DIR_AFTER}/* du -sh ${WHEEL_DIR_AFTER} ``` </details> ### devcontainers job? Once this PR is close to ready, let's merge the devcontainers PR and then re-run the devcontainers CI here. devcontainers PR: rapidsai/devcontainers#387 ### `rapids-metadata` changes? Not necessary, `libcuspatial` is already there: https://github.com/rapidsai/rapids-metadata/blob/9b6307e708511cd9a1990d8bb36606df53bc9e1b/src/rapids_metadata/__init__.py#L89 # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) - Mark Harris (https://github.com/harrism) - Vyas Ramasubramani (https://github.com/vyasr) URL: #1450
Fixes #1455 devcontainer conda CI jobs are failing in this project because of the following mix of characteristics for thoes jobs: * all build and runtime dependencies for `libcuspatial`, `cuspatial`, `cuproj` are installed via conda * `libcuspatial`, `cuspatial`, and `cuproj` wheels are then built with `pip install -e --no-deps --no-build-isolation` * `import libcuspatial` results in unconditionally running `import libcudf` * `libcudf` is provided by the `libcudf` **conda** package, which does not have any Python modules, so that import fails This fixes that, and restores the ability to mix a `pip install`'d `cuspatial` / `cuproj` with a `conda`-installed `libcudf`. ## Notes for Reviewers ### How did CI not catch this before? When rapidsai/devcontainers#387 was merged, I only re-ran the **pip** devcontainers CI job on #1450. # Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1456
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
…as>=1.0 (#1453) Contributes to rapidsai/build-planning#40 This PR adds support for Python 3.12. ## Notes for Reviewers This is part of ongoing work to add Python 3.12 support across RAPIDS. It temporarily introduces a build/test matrix including Python 3.12, from rapidsai/shared-workflows#213. A follow-up PR will revert back to pointing at the `branch-24.10` branch of `shared-workflows` once all RAPIDS repos have added Python 3.12 support. Other changes required to add that support: * pinning `proj` back to 9.3.x, reverting #1449 (#1453 (comment)) * requiring `geopandas >= 1.0` (#1453 (comment)) ### This will fail until all dependencies have been updates to Python 3.12 CI here is expected to fail until all of this project's upstream dependencies support Python 3.12. This can be merged whenever all CI jobs are passing. Authors: - James Lamb (https://github.com/jameslamb) - Bradley Dice (https://github.com/bdice) Approvers: - Bradley Dice (https://github.com/bdice) URL: #1453
Recommending `miniforge` for conda install in build instructions. Authors: - Mike McCarty (https://github.com/mmccarty) Approvers: - Mark Harris (https://github.com/harrism) URL: #1457
We need to update flake8 to fix a false-positive that appears with older flake8 versions on Python 3.12. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Matthew Roeschke (https://github.com/mroeschke) - James Lamb (https://github.com/jameslamb) URL: #1458
This PR removes the NumPy<2 pin which is expected to work for RAPIDS projects once CuPy 13.3.0 is released (CuPy 13.2.0 had some issues preventing the use with NumPy 2). Also contributes to rapidsai/build-planning#56 by removing the pin on `proj` (which allows this project to work with newer `fmt` / `spdlog`, in sync with conda-forge). Authors: - Sebastian Berg (https://github.com/seberg) - Bradley Dice (https://github.com/bdice) - James Lamb (https://github.com/jameslamb) - https://github.com/jakirkham Approvers: - Bradley Dice (https://github.com/bdice) URL: #1441
In cudf & cuml we have observed a ~10% to ~20% respectively speed up of pytest suite execution by switching pytest traceback to `--native`: ``` currently: 102474 passed, 2117 skipped, 902 xfailed in 892.16s (0:14:52) --tb=short: 102474 passed, 2117 skipped, 902 xfailed in 898.99s (0:14:58) --tb=no: 102474 passed, 2117 skipped, 902 xfailed in 815.98s (0:13:35) --tb=native: 102474 passed, 2117 skipped, 902 xfailed in 820.92s (0:13:40) ``` This PR makes similar change to `cuspatial` repo. xref: rapidsai/cudf#16851 Authors: - GALI PREM SAGAR (https://github.com/galipremsagar) Approvers: - Bradley Dice (https://github.com/bdice) - Paul Taylor (https://github.com/trxcllnt) URL: #1464
raydouglass
requested review from
jameslamb,
robertmaynard,
harrism,
bdice and
trxcllnt
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
github-actions
bot
added
conda
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.
❄️ Code freeze for
branch-24.10and v24.10 releaseWhat does this mean?
Only critical/hotfix level issues should be merged into
branch-24.10until release (merging of this PR).What is the purpose of this PR?
branch-24.10intomainfor the release