Adopt nvForest for random forest inference - #8048
Merged
Merged
Conversation
chyunsu3
marked this pull request as draft
May 5, 2026 05:01
github-actions
Bot
added
conda
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds nvforest/libnvforest to CI, build, packaging, and runtime; removes C++ FIL implementation, kernels, related raft_proto helpers, tests and benchmarks; switches Python RandomForest inference to nvforest and provides a deprecated FIL-compatible shim. ChangesnvForest integration and dependency wiring
C++ FIL removal, benchmarks, and tests cleanup
Python migration to nvForest and compatibility shims
Estimated code review effort Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cpp/CMakeLists.txt`:
- Around line 593-594: The cuml_objs target currently always links against
nvforest::nvforest++ (seen in the target_link_libraries PUBLIC list) but
nvforest is only created when LINK_NVFOREST is enabled; update the linking so
nvforest::nvforest++ is only added when available—either wrap the library entry
in a generator expression that tests target existence (e.g.
$<TARGET_EXISTS:nvforest::nvforest++>), or surround the target_link_libraries
entry with an if(LINK_NVFOREST) ... endif() check driven by the same flag set in
ConfigureAlgorithms.cmake; ensure you modify the cuml_objs target_link_libraries
invocation (the PUBLIC list containing rapids_logger::rapids_logger rmm::rmm
raft::raft nvforest::nvforest++ ${TREELITE_LIBS} ${_cuml_cpp_public_libs}) to
conditionally include nvforest::nvforest++.
In `@cpp/tests/CMakeLists.txt`:
- Line 48: Line unconditionally links nvforest::nvforest++ causing configure
failures when LINK_NVFOREST is OFF; wrap that library in a CMake generator
expression so it's only added when LINK_NVFOREST is true. Modify the
target_link_libraries entry that currently includes nvforest::nvforest++ to use
a conditional generator expression like
$<$<BOOL:${LINK_NVFOREST}>:nvforest::nvforest++> (matching the style used for
CUFFT/MPI/RAFT_DISTRIBUTED) so the nvforest target is only linked when
LINK_NVFOREST is enabled.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 062cb25e-c935-4869-ba7b-bd9da7e2ba31
📒 Files selected for processing (115)
conda/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.yamlcpp/CMakeLists.txtcpp/bench/CMakeLists.txtcpp/bench/sg/fil.cucpp/cmake/modules/ConfigureAlgorithms.cmakecpp/cmake/thirdparty/get_nvforest.cmakecpp/include/cuml/fil/Implementation.mdcpp/include/cuml/fil/README.mdcpp/include/cuml/fil/constants.hppcpp/include/cuml/fil/decision_forest.hppcpp/include/cuml/fil/detail/bitset.hppcpp/include/cuml/fil/detail/cpu_introspection.hppcpp/include/cuml/fil/detail/decision_forest_builder.hppcpp/include/cuml/fil/detail/degenerate_trees.hppcpp/include/cuml/fil/detail/device_initialization.hppcpp/include/cuml/fil/detail/device_initialization/cpu.hppcpp/include/cuml/fil/detail/device_initialization/gpu.cuhcpp/include/cuml/fil/detail/device_initialization/gpu.hppcpp/include/cuml/fil/detail/evaluate_tree.hppcpp/include/cuml/fil/detail/forest.hppcpp/include/cuml/fil/detail/gpu_introspection.hppcpp/include/cuml/fil/detail/index_type.hppcpp/include/cuml/fil/detail/infer.hppcpp/include/cuml/fil/detail/infer/cpu.hppcpp/include/cuml/fil/detail/infer/gpu.cuhcpp/include/cuml/fil/detail/infer/gpu.hppcpp/include/cuml/fil/detail/infer_kernel/cpu.hppcpp/include/cuml/fil/detail/infer_kernel/gpu.cuhcpp/include/cuml/fil/detail/infer_kernel/shared_memory_buffer.cuhcpp/include/cuml/fil/detail/node.hppcpp/include/cuml/fil/detail/postprocessor.hppcpp/include/cuml/fil/detail/raft_proto/buffer.hppcpp/include/cuml/fil/detail/raft_proto/ceildiv.hppcpp/include/cuml/fil/detail/raft_proto/cuda_check.hppcpp/include/cuml/fil/detail/raft_proto/cuda_stream.hppcpp/include/cuml/fil/detail/raft_proto/detail/const_agnostic.hppcpp/include/cuml/fil/detail/raft_proto/detail/copy.hppcpp/include/cuml/fil/detail/raft_proto/detail/copy/cpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/copy/gpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/cuda_check/base.hppcpp/include/cuml/fil/detail/raft_proto/detail/cuda_check/gpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/device_id/base.hppcpp/include/cuml/fil/detail/raft_proto/detail/device_id/cpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/device_id/gpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/device_setter/base.hppcpp/include/cuml/fil/detail/raft_proto/detail/device_setter/gpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/host_only_throw.hppcpp/include/cuml/fil/detail/raft_proto/detail/host_only_throw/base.hppcpp/include/cuml/fil/detail/raft_proto/detail/host_only_throw/cpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/non_owning_buffer.hppcpp/include/cuml/fil/detail/raft_proto/detail/non_owning_buffer/base.hppcpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer.hppcpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer/base.hppcpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer/cpu.hppcpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer/gpu.hppcpp/include/cuml/fil/detail/raft_proto/device_id.hppcpp/include/cuml/fil/detail/raft_proto/device_setter.hppcpp/include/cuml/fil/detail/raft_proto/device_type.hppcpp/include/cuml/fil/detail/raft_proto/exceptions.hppcpp/include/cuml/fil/detail/raft_proto/gpu_support.hppcpp/include/cuml/fil/detail/raft_proto/handle.hppcpp/include/cuml/fil/detail/raft_proto/padding.hppcpp/include/cuml/fil/detail/specialization_types.hppcpp/include/cuml/fil/detail/specializations/device_initialization_macros.hppcpp/include/cuml/fil/detail/specializations/forest_macros.hppcpp/include/cuml/fil/detail/specializations/infer_macros.hppcpp/include/cuml/fil/exceptions.hppcpp/include/cuml/fil/forest_model.hppcpp/include/cuml/fil/infer_kind.hppcpp/include/cuml/fil/postproc_ops.hppcpp/include/cuml/fil/tree_layout.hppcpp/include/cuml/fil/treelite_importer.hppcpp/src/fil/infer0.cppcpp/src/fil/infer0.cucpp/src/fil/infer1.cppcpp/src/fil/infer1.cucpp/src/fil/infer10.cppcpp/src/fil/infer10.cucpp/src/fil/infer11.cppcpp/src/fil/infer11.cucpp/src/fil/infer2.cppcpp/src/fil/infer2.cucpp/src/fil/infer3.cppcpp/src/fil/infer3.cucpp/src/fil/infer4.cppcpp/src/fil/infer4.cucpp/src/fil/infer5.cppcpp/src/fil/infer5.cucpp/src/fil/infer6.cppcpp/src/fil/infer6.cucpp/src/fil/infer7.cppcpp/src/fil/infer7.cucpp/src/fil/infer8.cppcpp/src/fil/infer8.cucpp/src/fil/infer9.cppcpp/src/fil/infer9.cucpp/tests/CMakeLists.txtcpp/tests/sg/fil/decision_forest_builder_invalid_inputs.cppcpp/tests/sg/fil/raft_proto/buffer.cppcpp/tests/sg/fil/raft_proto/buffer.cucpp/tests/sg/fil/treelite_importer.cppcpp/tests/sg/fil/treelite_importer_invalid_inputs.cppcpp/tests/sg/forest/traversal_forest.cppcpp/tests/sg/forest/treelite_traversal.cppcpp/tests/sg/rf_test.cudependencies.yamlpython/cuml/pyproject.tomlpython/libcuml/pyproject.toml
💤 Files with no reviewable changes (98)
- cpp/include/cuml/fil/Implementation.md
- cpp/src/fil/infer1.cpp
- cpp/include/cuml/fil/detail/cpu_introspection.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/non_owning_buffer.hpp
- cpp/include/cuml/fil/detail/raft_proto/device_id.hpp
- cpp/src/fil/infer11.cpp
- cpp/src/fil/infer11.cu
- cpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/device_setter/gpu.hpp
- cpp/include/cuml/fil/detail/device_initialization/cpu.hpp
- cpp/include/cuml/fil/detail/raft_proto/device_setter.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/const_agnostic.hpp
- cpp/include/cuml/fil/tree_layout.hpp
- cpp/src/fil/infer7.cpp
- cpp/src/fil/infer2.cu
- cpp/include/cuml/fil/detail/raft_proto/cuda_stream.hpp
- cpp/src/fil/infer10.cu
- cpp/src/fil/infer4.cu
- cpp/tests/sg/forest/treelite_traversal.cpp
- cpp/include/cuml/fil/detail/evaluate_tree.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/host_only_throw.hpp
- cpp/include/cuml/fil/detail/device_initialization/gpu.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/device_id/base.hpp
- cpp/include/cuml/fil/detail/specializations/device_initialization_macros.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/copy/gpu.hpp
- cpp/include/cuml/fil/detail/node.hpp
- cpp/include/cuml/fil/detail/raft_proto/exceptions.hpp
- cpp/include/cuml/fil/detail/gpu_introspection.hpp
- cpp/src/fil/infer6.cu
- cpp/include/cuml/fil/detail/raft_proto/detail/device_id/gpu.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer/cpu.hpp
- cpp/src/fil/infer0.cu
- cpp/include/cuml/fil/detail/infer_kernel/cpu.hpp
- cpp/src/fil/infer10.cpp
- cpp/src/fil/infer0.cpp
- cpp/include/cuml/fil/detail/raft_proto/device_type.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/host_only_throw/cpu.hpp
- cpp/src/fil/infer5.cpp
- cpp/include/cuml/fil/exceptions.hpp
- cpp/include/cuml/fil/constants.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/cuda_check/gpu.hpp
- cpp/src/fil/infer7.cu
- cpp/include/cuml/fil/detail/index_type.hpp
- cpp/include/cuml/fil/detail/postprocessor.hpp
- cpp/include/cuml/fil/detail/bitset.hpp
- cpp/tests/sg/forest/traversal_forest.cpp
- cpp/include/cuml/fil/detail/specialization_types.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/host_only_throw/base.hpp
- cpp/include/cuml/fil/detail/raft_proto/ceildiv.hpp
- cpp/include/cuml/fil/detail/forest.hpp
- cpp/src/fil/infer3.cpp
- cpp/include/cuml/fil/detail/specializations/forest_macros.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/device_id/cpu.hpp
- cpp/bench/sg/fil.cu
- cpp/include/cuml/fil/README.md
- cpp/include/cuml/fil/detail/raft_proto/detail/device_setter/base.hpp
- cpp/src/fil/infer9.cpp
- cpp/include/cuml/fil/detail/device_initialization.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/copy/cpu.hpp
- cpp/include/cuml/fil/infer_kind.hpp
- cpp/include/cuml/fil/treelite_importer.hpp
- cpp/tests/sg/fil/raft_proto/buffer.cpp
- cpp/src/fil/infer5.cu
- cpp/src/fil/infer6.cpp
- cpp/include/cuml/fil/detail/infer/gpu.hpp
- cpp/include/cuml/fil/detail/degenerate_trees.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer/base.hpp
- cpp/include/cuml/fil/decision_forest.hpp
- cpp/tests/sg/fil/treelite_importer.cpp
- cpp/include/cuml/fil/detail/raft_proto/detail/non_owning_buffer/base.hpp
- cpp/include/cuml/fil/postproc_ops.hpp
- cpp/tests/sg/fil/raft_proto/buffer.cu
- cpp/include/cuml/fil/detail/raft_proto/padding.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/copy.hpp
- cpp/src/fil/infer4.cpp
- cpp/src/fil/infer8.cu
- cpp/include/cuml/fil/detail/decision_forest_builder.hpp
- cpp/include/cuml/fil/detail/specializations/infer_macros.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/cuda_check/base.hpp
- cpp/tests/sg/fil/treelite_importer_invalid_inputs.cpp
- cpp/include/cuml/fil/detail/raft_proto/cuda_check.hpp
- cpp/include/cuml/fil/detail/raft_proto/detail/owning_buffer/gpu.hpp
- cpp/src/fil/infer2.cpp
- cpp/include/cuml/fil/detail/device_initialization/gpu.cuh
- cpp/tests/sg/fil/decision_forest_builder_invalid_inputs.cpp
- cpp/include/cuml/fil/detail/infer_kernel/shared_memory_buffer.cuh
- cpp/include/cuml/fil/detail/raft_proto/buffer.hpp
- cpp/src/fil/infer9.cu
- cpp/include/cuml/fil/detail/infer/gpu.cuh
- cpp/include/cuml/fil/detail/infer_kernel/gpu.cuh
- cpp/include/cuml/fil/detail/infer.hpp
- cpp/src/fil/infer3.cu
- cpp/src/fil/infer1.cu
- cpp/include/cuml/fil/detail/raft_proto/handle.hpp
- cpp/src/fil/infer8.cpp
- cpp/include/cuml/fil/detail/infer/cpu.hpp
- cpp/include/cuml/fil/detail/raft_proto/gpu_support.hpp
- cpp/include/cuml/fil/forest_model.hpp
chyunsu3
added
improvement
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
python/cuml/cuml/fil/compat.py (1)
309-314:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winMissing
stacklevel=2in deprecation warning.The
load_from_treelite_modelclassmethod's deprecation warning is missingstacklevel=2, unlike the other classmethods (loadat line 231,load_from_sklearnat line 273). This causes the warning traceback to point to this library code rather than the user's calling code.Suggested fix
warnings.warn( "cuml.fil.ForestInference.load_from_treelite_model() is deprecated " "and will be removed in 26.10. " "Use nvforest.load_from_treelite_model() instead.", FutureWarning, + stacklevel=2, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuml/cuml/fil/compat.py` around lines 309 - 314, The deprecation warning in the classmethod load_from_treelite_model (cuml.fil.ForestInference.load_from_treelite_model) is missing stacklevel=2; update the warnings.warn call in that method to pass stacklevel=2 so the warning points at the user's calling code (mirror the existing usage in load and load_from_sklearn).
🧹 Nitpick comments (1)
python/cuml/tests/test_random_forest.py (1)
754-757: ⚡ Quick winInconsistent array conversion between classification and regression tests.
The classification test (lines 695-697) explicitly converts the nvforest output to NumPy via
cp.asnumpy(), but the regression test here passes the nvforest output directly tonp.reshape()without conversion. While this may work due to array protocols, it's inconsistent with the classification test pattern and could cause issues depending on the output type.Consider using explicit conversion for consistency:
Suggested fix
nvforest_model = cuml_model.as_nvforest() - nvforest_model_preds = nvforest_model.predict(X_test) - nvforest_model_preds = np.reshape(nvforest_model_preds, np.shape(y_test)) + nvforest_model_preds = cp.asnumpy(nvforest_model.predict(X_test)) + nvforest_model_preds = np.reshape(nvforest_model_preds, np.shape(y_test))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuml/tests/test_random_forest.py` around lines 754 - 757, The regression test is inconsistently handling nvforest outputs compared to the classification test: after calling nvforest_model.predict(X_test) convert the result to a NumPy array (e.g., via cp.asnumpy()) before reshaping and computing r2_score so nvforest_model_preds is the same concrete type as in the classification test; update the nvforest_model.predict usage and subsequent np.reshape on nvforest_model_preds to explicitly convert to NumPy prior to evaluating nvforest_model_r2.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@python/cuml/cuml/fil/compat.py`:
- Around line 309-314: The deprecation warning in the classmethod
load_from_treelite_model (cuml.fil.ForestInference.load_from_treelite_model) is
missing stacklevel=2; update the warnings.warn call in that method to pass
stacklevel=2 so the warning points at the user's calling code (mirror the
existing usage in load and load_from_sklearn).
---
Nitpick comments:
In `@python/cuml/tests/test_random_forest.py`:
- Around line 754-757: The regression test is inconsistently handling nvforest
outputs compared to the classification test: after calling
nvforest_model.predict(X_test) convert the result to a NumPy array (e.g., via
cp.asnumpy()) before reshaping and computing r2_score so nvforest_model_preds is
the same concrete type as in the classification test; update the
nvforest_model.predict usage and subsequent np.reshape on nvforest_model_preds
to explicitly convert to NumPy prior to evaluating nvforest_model_r2.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 74300b27-5a68-4b92-99be-1a26975f3925
📒 Files selected for processing (5)
docs/source/FIL.rstpython/cuml/cuml/ensemble/randomforest_common.pyxpython/cuml/cuml/fil/compat.pypython/cuml/tests/test_fil.pypython/cuml/tests/test_random_forest.py
💤 Files with no reviewable changes (1)
- docs/source/FIL.rst
csadorf
reviewed
May 21, 2026
csadorf
changed the title
bdice
requested changes
May 21, 2026
bdice
left a comment
bdice
left a comment
Contributor
There was a problem hiding this comment.
Additional changes needed:
-
Update
ci/release/update-version.shto include nvforest and libnvforest in the DEPENDENCIES list. Once this PR merges, you must rerun that script withci/release/update-version.sh 26.08.00and commit as part of the manual forward-merge! -
Add nvforest as a cuml dependency in the devcontainers (this can happen now, no need to wait for this PR): https://github.com/rapidsai/devcontainers/blob/aea286b1c921f8e50a598551e0353bb867a00258/features/src/rapids-build-utils/opt/rapids-build-utils/manifest.yaml#L197
jcrist
approved these changes
May 21, 2026
jcrist
left a comment
jcrist
left a comment
Member
There was a problem hiding this comment.
The tiniest of docs nits, otherwise LGTM from a python perspective!
There was a problem hiding this comment.
♻️ Duplicate comments (1)
python/cuml/cuml/ensemble/randomforest_common.pyx (1)
407-410:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the runtime deprecation warning aligned with the documented removal release.
The docstring now says
as_filis removed in 26.08, but the emittedFutureWarningstill says 26.10. That leaves users with conflicting guidance from the same API surface.Proposed fix
warnings.warn( - "as_fil() method is deprecated and will be removed in 26.10. " + "as_fil() method is deprecated and will be removed in 26.08. " "Use the as_nvforest() method instead.", FutureWarning, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuml/cuml/ensemble/randomforest_common.pyx` around lines 407 - 410, Update the FutureWarning emitted in the as_fil() deprecation to match the documented removal release (26.08): locate the warnings.warn(...) call inside the as_fil method in randomforest_common.pyx and change the message text from "will be removed in 26.10" to "will be removed in 26.08" (while keeping the rest of the message and FutureWarning class unchanged); also ensure any adjacent docstring or comment in the same method mentioning as_fil/as_nvforest remains consistent with the new release string.
🧹 Nitpick comments (1)
python/cuml/cuml/ensemble/randomforest_common.pyx (1)
680-681: ⚡ Quick winReuse the cached nvForest model in the OOB path.
_compute_oob_score()callsas_nvforest()directly, sooob_score=Truebuilds a fresh inference model and immediately throws it away instead of warming_nvforest_model. Using_get_inference_nvforest_model()here avoids an extra Treelite→nvForest load on the next inference call.Proposed fix
- nvforest_model = self.as_nvforest() + nvforest_model = self._get_inference_nvforest_model()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/cuml/cuml/ensemble/randomforest_common.pyx` around lines 680 - 681, The OOB path in _compute_oob_score currently calls as_nvforest() which builds a fresh nvForest model and discards it; replace that call with _get_inference_nvforest_model() so the code reuses and warms the cached _nvforest_model (when oob_score=True) and avoids an extra Treelite→nvForest load on subsequent inferences.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@python/cuml/cuml/ensemble/randomforest_common.pyx`:
- Around line 407-410: Update the FutureWarning emitted in the as_fil()
deprecation to match the documented removal release (26.08): locate the
warnings.warn(...) call inside the as_fil method in randomforest_common.pyx and
change the message text from "will be removed in 26.10" to "will be removed in
26.08" (while keeping the rest of the message and FutureWarning class
unchanged); also ensure any adjacent docstring or comment in the same method
mentioning as_fil/as_nvforest remains consistent with the new release string.
---
Nitpick comments:
In `@python/cuml/cuml/ensemble/randomforest_common.pyx`:
- Around line 680-681: The OOB path in _compute_oob_score currently calls
as_nvforest() which builds a fresh nvForest model and discards it; replace that
call with _get_inference_nvforest_model() so the code reuses and warms the
cached _nvforest_model (when oob_score=True) and avoids an extra
Treelite→nvForest load on subsequent inferences.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: abc1f576-a70c-4084-be8d-7c86aa765c73
📒 Files selected for processing (4)
ci/build_wheel_cuml.shci/build_wheel_libcuml.shci/release/update-version.shpython/cuml/cuml/ensemble/randomforest_common.pyx
bdice
approved these changes
May 21, 2026
Contributor
|
/merge |
trxcllnt
added a commit
to rapidsai/devcontainers
that referenced
this pull request
May 22, 2026
Add `nvforest` to the cuML C++ devcontainer dependency list so cuML builds after nvForest when using the RAPIDS build-utils manifest. Follow-up to NVIDIA/cuml#8048 --------- Co-authored-by: ptaylor <paul.e.taylor@me.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Adopts
nvForestfor cuML random forest inference, removes the in-tree FIL implementation, and keeps a deprecatedcuml.filcompatibility layer that directs users tonvforest.ForestInference.This also wires
nvForestinto the C++ and Python build paths, updates packaging dependencies, and preserves the existingcuml.fil.ForestInferenceentry point with deprecation warnings for users migrating from FIL.Checklist
dependencies.yamlCMakeLists.txtto build withnvForestnvForestnvForestcuml.fil.ForestInferenceto throw a warning; direct users to usenvforest.ForestInferenceinstead.