Forward-merge release/26.04 into main - #89
Conversation
Follow-up to rapidsai/devcontainers#672 Authors: - Philip Hyunsu Cho (https://github.com/hcho3) - James Lamb (https://github.com/jameslamb) Approvers: - James Lamb (https://github.com/jameslamb) URL: rapidsai#82
…s, drop CUDA math libraries dependencies (rapidsai#87) Contributes to rapidsai/build-planning#257 * builds CUDA 13 wheels with the 13.0 CTK Contributes to rapidsai/build-planning#256 * updates wheel tests to cover a range of CTK versions (we previously, accidentally, were only testing the latest 12.x and 13.x) Drops unnecessary CTK dependencies * drops dependencies on cuBLAS, cuFFT, cuRAND, cuSOLVER, and cuSPARSE - *`nvforest` doesn't depend directly on any of these, I suspect they were all just copied over from cuML* * removes related code and configuration Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Philip Hyunsu Cho (https://github.com/hcho3) - Gil Forsyth (https://github.com/gforsyth) URL: rapidsai#87
`packaging` is only used in docs builds: https://github.com/rapidsai/nvforest/blob/1dcd17c74f7b03a64a030d5d7460bc3351dd13b1/docs/source/conf.py#L24 and tests: https://github.com/rapidsai/nvforest/blob/1dcd17c74f7b03a64a030d5d7460bc3351dd13b1/python/nvforest/tests/test_nvforest.py#L763 It doesn't need to be declared as a runtime dependency of `nvforest`. This moves it to a test-time-only dependency. Authors: - James Lamb (https://github.com/jameslamb) Approvers: - Philip Hyunsu Cho (https://github.com/hcho3) - Gil Forsyth (https://github.com/gforsyth) URL: rapidsai#88
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR introduces development container support for VS Code and GitHub Codespaces with multiple CUDA/package manager variants, enables devcontainer testing in CI, removes explicit CUDA math library dependencies from conda environments and build configurations, and restructures Python project dependencies to remove cuda-toolkit and packaging from core requirements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.devcontainer/cuda12.9-conda/devcontainer.json (2)
29-35: Note:consistencyoption is deprecated in recent Docker versions.The
consistency=consistentoption for bind mounts is deprecated and has no effect on Linux systems. It was primarily used for macOS performance tuning in older Docker versions. While this doesn't cause issues, it can be safely removed to reduce noise:- "source=${localWorkspaceFolder}/../.aws,target=/home/coder/.aws,type=bind,consistency=consistent", + "source=${localWorkspaceFolder}/../.aws,target=/home/coder/.aws,type=bind",Apply similar changes to all mount entries.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/cuda12.9-conda/devcontainer.json around lines 29 - 35, Remove the deprecated "consistency=consistent" option from the "mounts" array entries in the devcontainer.json; locate the "mounts" array and each bind mount string (those containing "source=${localWorkspaceFolder}/../..." and "type=bind") and strip the ",consistency=consistent" fragment from each mount entry so the mounts remain valid on Linux and avoid deprecated options.
28-28: Consider using a dynamic workspace name for flexibility.The workspace mount hardcodes
nvforestas the target directory. If developers clone the repository with a different name, this could cause confusion. Consider using${localWorkspaceFolderBasename}instead:- "workspaceMount": "source=${localWorkspaceFolder},target=/home/coder/nvforest,type=bind,consistency=consistent", + "workspaceMount": "source=${localWorkspaceFolder},target=/home/coder/${localWorkspaceFolderBasename},type=bind,consistency=consistent",However, if a consistent internal path is desired regardless of the local clone name, the current approach is acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/cuda12.9-conda/devcontainer.json at line 28, The workspaceMount currently hardcodes the target path as /home/coder/nvforest which can mismatch local clone names; change the mount target to use the variable ${localWorkspaceFolderBasename} (i.e., update the "workspaceMount" value) so the container path dynamically reflects the local repository folder name, or document/keep the hardcoded /home/coder/nvforest if a fixed internal path is intentionally required.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.devcontainer/cuda12.9-conda/devcontainer.json:
- Line 20: The RAPIDS devcontainer feature is pinned to
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26.4" but should
match the RAPIDS 26.04 base image which expects version ":26"; update the
feature string in each devcontainer configuration (symbols to locate:
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26.4") to use
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26" in the
cuda12.9-conda, cuda12.9-pip, cuda13.1-conda, and cuda13.1-pip devcontainer.json
files.
In @.devcontainer/README.md:
- Line 33: The image tag in .devcontainer/README.md is missing an alt attribute;
update the <img> element to include a concise, descriptive alt text (e.g.,
alt="VS Code: Reopen in Container button highlighted") so screen readers can
convey the image meaning and improve accessibility.
---
Nitpick comments:
In @.devcontainer/cuda12.9-conda/devcontainer.json:
- Around line 29-35: Remove the deprecated "consistency=consistent" option from
the "mounts" array entries in the devcontainer.json; locate the "mounts" array
and each bind mount string (those containing
"source=${localWorkspaceFolder}/../..." and "type=bind") and strip the
",consistency=consistent" fragment from each mount entry so the mounts remain
valid on Linux and avoid deprecated options.
- Line 28: The workspaceMount currently hardcodes the target path as
/home/coder/nvforest which can mismatch local clone names; change the mount
target to use the variable ${localWorkspaceFolderBasename} (i.e., update the
"workspaceMount" value) so the container path dynamically reflects the local
repository folder name, or document/keep the hardcoded /home/coder/nvforest if a
fixed internal path is intentionally required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6c11cc04-ce9e-425b-9a9d-76b9de9729ad
📒 Files selected for processing (23)
.devcontainer/Dockerfile.devcontainer/README.md.devcontainer/cuda12.9-conda/devcontainer.json.devcontainer/cuda12.9-pip/devcontainer.json.devcontainer/cuda13.1-conda/devcontainer.json.devcontainer/cuda13.1-pip/devcontainer.json.github/workflows/pr.yamlci/test_wheel.shconda/environments/all_cuda-129_arch-aarch64.yamlconda/environments/all_cuda-129_arch-x86_64.yamlconda/environments/all_cuda-131_arch-aarch64.yamlconda/environments/all_cuda-131_arch-x86_64.yamlconda/environments/clang_tidy_cuda-129_arch-x86_64.yamlconda/environments/clang_tidy_cuda-131_arch-x86_64.yamlconda/environments/cpp_all_cuda-129_arch-x86_64.yamlconda/environments/cpp_all_cuda-131_arch-x86_64.yamlconda/recipes/libnvforest/recipe.yamlconda/recipes/nvforest/recipe.yamlcpp/CMakeLists.txtdependencies.yamlpython/libnvforest/CMakeLists.txtpython/libnvforest/pyproject.tomlpython/nvforest/pyproject.toml
💤 Files with no reviewable changes (12)
- conda/environments/clang_tidy_cuda-129_arch-x86_64.yaml
- conda/environments/all_cuda-131_arch-aarch64.yaml
- conda/environments/all_cuda-129_arch-x86_64.yaml
- conda/environments/clang_tidy_cuda-131_arch-x86_64.yaml
- conda/recipes/nvforest/recipe.yaml
- conda/environments/cpp_all_cuda-131_arch-x86_64.yaml
- conda/environments/cpp_all_cuda-129_arch-x86_64.yaml
- python/libnvforest/pyproject.toml
- conda/environments/all_cuda-131_arch-x86_64.yaml
- conda/environments/all_cuda-129_arch-aarch64.yaml
- python/libnvforest/CMakeLists.txt
- conda/recipes/libnvforest/recipe.yaml
| ], | ||
| "hostRequirements": {"gpu": "optional"}, | ||
| "features": { | ||
| "ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26.4": {} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if the rapids-build-utils feature version 26.4 exists and matches the 26.04 release
# Query the GitHub Container Registry for available feature versions
gh api /orgs/rapidsai/packages/container/devcontainers%2Ffeatures%2Frapids-build-utils/versions --jq '.[] | select(.name | startswith("26")) | .name' | head -10Repository: rapidsai/nvforest
Length of output: 221
🏁 Script executed:
# First, explore the .devcontainer directory structure
find .devcontainer -type f -name "*.json" | head -20Repository: rapidsai/nvforest
Length of output: 244
🏁 Script executed:
# Check the actual content of the file in question
cat -n .devcontainer/cuda12.9-conda/devcontainer.json 2>/dev/null || echo "File not found"Repository: rapidsai/nvforest
Length of output: 2320
🏁 Script executed:
# Look for other devcontainer configurations that might use similar features
grep -r "rapids-build-utils" .devcontainer/ 2>/dev/null || echo "No matches found"Repository: rapidsai/nvforest
Length of output: 984
🌐 Web query:
RAPIDS devcontainers features rapids-build-utils version 26.4
💡 Result:
RAPIDS publishes rapids-build-utils as a Dev Container Feature on GHCR with a 26.4 tag (and patch tags like 26.4.0, 26.4.1, …). The tagged versions are listed on the GitHub Container Registry package page. [1]
Use in devcontainer.json (pin to 26.4)
{
"features": {
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26.4": {}
}
}(You can also pin to an exact patch, e.g. :26.4.0.) [1]
What version 26.4.0 does
- Sets
BASH_ENV=/etc/bash.bash_envand runsrapids-post-start-commandas the featurepostStartCommand. [2] - Installs common OS packages (via apt):
bc jq pigz sudo wget gettext-base bash-completion ca-certificates(pluspython3/python3-pipif needed). [3] - Installs
yq(v4.x) if missing. [3] - Uses
pipto install:rapids-dependency-file-generator,conda-merge, andtoml. [3] - Copies RAPIDS build utility scripts into
/opt/rapids-build-utilsand registers many helper commands asrapids-*(viaupdate-alternatives), e.g.rapids-make-conda-env,rapids-make-pip-env,rapids-merge-compile-commands-json, etc. [3] rapids-post-start-commandtriggers typical “devcontainer bootstrap” tasks (generate scripts, update build-dir links, update VS Code workspace, merge compile commands; and optionally create a Python env based onPYTHON_PACKAGE_MANAGER). [4]
Source mapping: [1] GHCR package versions list; [2] devcontainer-feature.json for tag v26.04.00a (feature version 26.4.0); [3] install.sh for 26.4.0; [4] post-start-command.sh for 26.4.0.
🌐 Web query:
RAPIDS 26.04 release devcontainer feature versions
💡 Result:
For the RAPIDS 26.04 devcontainers repo tag v26.04.00a, the devcontainer Feature “version” you pin to is :26 (the major CalVer), e.g. ghcr.io/rapidsai/devcontainers/features/cuda:26. [1]
Available RAPIDS devcontainer Features in that release (all used as .../features/<id>:26) are: cccl-dev, cmake, cuda, gcc, gitlab-cli, llvm, mambaforge, ninja, nvhpc, oneapi, openmpi, rapids-build-utils, rust, sccache, ucx, utils. [2]
Example
"features": {
"ghcr.io/rapidsai/devcontainers/features/cuda:26": {},
"ghcr.io/rapidsai/devcontainers/features/cmake:26": {}
}
``` [1][3]
**Sources:** RAPIDS devcontainers tag `v26.04.00a` (alpha) feature docs/listing. [1][2][3]
---
</details>
**Use feature version `:26` to match the RAPIDS 26.04 base image.**
The feature is pinned to `rapids-build-utils:26.4`, but the base image uses RAPIDS 26.04 which references feature version `:26`. Change line 20 to:"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26": {}
This affects all four devcontainer configurations (cuda12.9-conda, cuda12.9-pip, cuda13.1-conda, cuda13.1-pip).
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/cuda12.9-conda/devcontainer.json at line 20, The RAPIDS
devcontainer feature is pinned to
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26.4" but should
match the RAPIDS 26.04 base image which expects version ":26"; update the
feature string in each devcontainer configuration (symbols to locate:
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26.4") to use
"ghcr.io/rapidsai/devcontainers/features/rapids-build-utils:26" in the
cuda12.9-conda, cuda12.9-pip, cuda13.1-conda, and cuda13.1-pip devcontainer.json
files.
</details>
<!-- fingerprinting:phantom:triton:puma -->
<!-- This is an auto-generated comment by CodeRabbit -->
|
|
||
| ## Launch a Dev Container | ||
|
|
||
| To launch a devcontainer from VSCode, open the nvForest repo and select the "Reopen in Container" button in the bottom right:<br/><img src="https://user-images.githubusercontent.com/178183/221771999-97ab29d5-e718-4e5f-b32f-2cdd51bba25c.png"/> |
There was a problem hiding this comment.
Add alt text to the image for accessibility.
The image is missing alternative text, which is important for screen readers and accessibility compliance.
📝 Proposed fix
-To launch a devcontainer from VSCode, open the nvForest repo and select the "Reopen in Container" button in the bottom right:<br/><img src="https://user-images.githubusercontent.com/178183/221771999-97ab29d5-e718-4e5f-b32f-2cdd51bba25c.png"/>
+To launch a devcontainer from VSCode, open the nvForest repo and select the "Reopen in Container" button in the bottom right:<br/><img src="https://user-images.githubusercontent.com/178183/221771999-97ab29d5-e718-4e5f-b32f-2cdd51bba25c.png" alt="VSCode Reopen in Container button"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| To launch a devcontainer from VSCode, open the nvForest repo and select the "Reopen in Container" button in the bottom right:<br/><img src="https://user-images.githubusercontent.com/178183/221771999-97ab29d5-e718-4e5f-b32f-2cdd51bba25c.png"/> | |
| To launch a devcontainer from VSCode, open the nvForest repo and select the "Reopen in Container" button in the bottom right:<br/><img src="https://user-images.githubusercontent.com/178183/221771999-97ab29d5-e718-4e5f-b32f-2cdd51bba25c.png" alt="VSCode Reopen in Container button"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 33-33: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.devcontainer/README.md at line 33, The image tag in .devcontainer/README.md
is missing an alt attribute; update the <img> element to include a concise,
descriptive alt text (e.g., alt="VS Code: Reopen in Container button
highlighted") so screen readers can convey the image meaning and improve
accessibility.
|
Nightly tests have been failing because of missing XGBoost packages Should be fixed by these:
I'll try manually triggering nightly tests here to get that CI job working again. |
|
All nightly tests except 1 (v100 + driver v3535) passed: https://github.com/rapidsai/nvforest/actions/runs/23354547701/job/67941927712 Wrote that up here: #90 So |
Closes #86
Fixes forward-merger conflicts