Skip to content

[DO NOT MERGE] Test rapidsai/nvforest#219 - #8601

Open
chyunsu3 wants to merge 7 commits into
NVIDIA:mainfrom
chyunsu3:update_nvforest
Open

[DO NOT MERGE] Test rapidsai/nvforest#219#8601
chyunsu3 wants to merge 7 commits into
NVIDIA:mainfrom
chyunsu3:update_nvforest

Conversation

@chyunsu3

@chyunsu3 chyunsu3 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Companion to rapidsai/nvforest#219

@chyunsu3
chyunsu3 requested a review from a team as a code owner September 10, 2026 02:03
@github-actions github-actions Bot added the ci label Sep 10, 2026
@chyunsu3 chyunsu3 added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 10, 2026
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: be0b7f72-78f7-41a6-b63e-de1fe0d397ff

📥 Commits

Reviewing files that changed from the base of the PR and between bc091a1 and 26ca86e.

📒 Files selected for processing (2)
  • cpp/cmake/thirdparty/get_nvforest.cmake
  • python/cuml/cuml/ensemble/isolation_forest.pyx

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Bug Fixes

    • Improved CUDA stream handling when exporting random forest and isolation forest models to NVForest, supporting more reliable predictions and model operations.
  • Chores

    • Improved automated build and test workflows by supporting pull-request versions of NVForest Conda packages and wheels.
    • Updated dependency resolution and package channel prioritization across C++, Python, documentation, notebook, and wheel workflows.
    • Updated the NVForest dependency source and refreshed copyright year ranges in affected workflows.

Walkthrough

Changes

CI scripts retrieve pull-request nvforest Conda packages and wheels. Build and test flows consume them. nvforest prediction and model conversion now pass explicit CUDA streams. The nvforest dependency uses a pinned fork commit.

nvforest integration

Layer / File(s) Summary
Conda package selection and consumers
ci/use_conda_packages_from_prs.sh, ci/build_*.sh, ci/test_cpp.sh, ci/test_notebooks.sh, ci/test_python_common.sh
The Conda helper retrieves pull-request artifacts and configures channels for builds and tests.
Wheel selection and consumers
ci/use_wheels_from_prs.sh, ci/build_wheel_*.sh, ci/test_wheel*.sh
The wheel helper retrieves pull-request wheels and writes pip constraints for wheel builds and tests.
Explicit CUDA stream usage
cpp/tests/sg/rf_test.cu, python/cuml/cuml/ensemble/*.pyx, cpp/cmake/thirdparty/get_nvforest.cmake
Prediction tests and Python model conversion pass explicit CUDA streams to nvforest. The dependency configuration selects a pinned fork commit.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: betatim

Merge Risk: ⚪ Minimal · up to 1a459

The reviewed nvForest CI and CUDA-stream integration changes have no identified merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 14 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies that this pull request tests changes from rapidsai/nvforest#219. It is concise and matches the stated objectives.
Description check ✅ Passed The description accurately summarizes the API updates and CI validation with artifacts from rapidsai/nvforest#219.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 14 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@chyunsu3
chyunsu3 requested review from a team as code owners September 10, 2026 02:54
@chyunsu3
chyunsu3 requested a review from jcrist September 10, 2026 02:54
@github-actions github-actions Bot added Cython / Python Cython or Python issue CUDA/C++ labels Sep 10, 2026
@chyunsu3
chyunsu3 requested a review from a team as a code owner September 11, 2026 02:23
@github-actions github-actions Bot added the CMake label Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CMake CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants