Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cpp/include/cuml/tree/decisiontree.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2019-2023, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/

Expand All @@ -17,7 +17,8 @@ namespace DT {

struct DecisionTreeParams {
/**
* Maximum tree depth. Unlimited (e.g., until leaves are pure), If `-1`.
* Maximum tree depth. Set to INT32_MAX for unlimited depth
* (i.e., until leaves are pure or other stopping criteria are met).
Comment thread
coderabbitai[bot] marked this conversation as resolved.
*/
int max_depth;
/**
Expand Down
6 changes: 2 additions & 4 deletions cpp/src/decisiontree/batched-levelalgo/builder.cuh
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -468,8 +468,7 @@ struct Builder {

// create child nodes (or make the current ones leaf)
raft::common::nvtx::push_range("nodeSplitKernel @builder.cuh [batched-levelalgo]");
launchNodeSplitKernel<DataT, LabelT, IdxT, TPB_DEFAULT>(params.max_depth,
params.min_samples_leaf,
launchNodeSplitKernel<DataT, LabelT, IdxT, TPB_DEFAULT>(params.min_samples_leaf,
params.min_samples_split,
params.max_leaves,
params.min_impurity_decrease,
Expand Down Expand Up @@ -525,7 +524,6 @@ struct Builder {
raft::common::nvtx::range kernel_scope("computeSplitKernel @builder.cuh [batched-levelalgo]");
launchComputeSplitKernel<DataT, LabelT, IdxT, TPB_DEFAULT>(histograms,
params.max_n_bins,
params.max_depth,
params.min_samples_split,
params.max_leaves,
dataset,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -64,8 +64,7 @@ DI OutT* alignPointer(InT dataset)
}

template <typename DataT, typename LabelT, typename IdxT, int TPB>
void launchNodeSplitKernel(const IdxT max_depth,
const IdxT min_samples_leaf,
void launchNodeSplitKernel(const IdxT min_samples_leaf,
const IdxT min_samples_split,
const IdxT max_leaves,
const DataT min_impurity_decrease,
Expand Down Expand Up @@ -388,7 +387,6 @@ template <typename DataT,
typename BinT>
void launchComputeSplitKernel(BinT* histograms,
IdxT n_bins,
IdxT max_depth,
IdxT min_samples_split,
IdxT max_leaves,
const Dataset<DataT, LabelT, IdxT>& dataset,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
* SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION.
* SPDX-License-Identifier: Apache-2.0
*/
#pragma once
Expand Down Expand Up @@ -78,8 +78,7 @@ DI void partitionSamples(const Dataset<DataT, LabelT, IdxT>& dataset,
}
}
template <typename DataT, typename LabelT, typename IdxT, int TPB>
static __global__ void nodeSplitKernel(const IdxT max_depth,
const IdxT min_samples_leaf,
static __global__ void nodeSplitKernel(const IdxT min_samples_leaf,
const IdxT min_samples_split,
const IdxT max_leaves,
const DataT min_impurity_decrease,
Expand All @@ -98,8 +97,7 @@ static __global__ void nodeSplitKernel(const IdxT max_depth,
}

template <typename DataT, typename LabelT, typename IdxT, int TPB>
void launchNodeSplitKernel(const IdxT max_depth,
const IdxT min_samples_leaf,
void launchNodeSplitKernel(const IdxT min_samples_leaf,
const IdxT min_samples_split,
const IdxT max_leaves,
const DataT min_impurity_decrease,
Expand All @@ -111,8 +109,7 @@ void launchNodeSplitKernel(const IdxT max_depth,
{
auto constexpr smem_size = 2 * sizeof(IdxT) * TPB;
nodeSplitKernel<DataT, LabelT, IdxT, TPB>
<<<work_items_size, TPB, smem_size, builder_stream>>>(max_depth,
min_samples_leaf,
<<<work_items_size, TPB, smem_size, builder_stream>>>(min_samples_leaf,
min_samples_split,
max_leaves,
min_impurity_decrease,
Expand Down Expand Up @@ -204,7 +201,6 @@ template <typename DataT,
typename BinT>
static __global__ void computeSplitKernel(BinT* histograms,
IdxT max_n_bins,
IdxT max_depth,
IdxT min_samples_split,
IdxT max_leaves,
const Dataset<DataT, LabelT, IdxT> dataset,
Expand Down Expand Up @@ -337,7 +333,6 @@ template <typename DataT,
typename BinT>
void launchComputeSplitKernel(BinT* histograms,
IdxT max_n_bins,
IdxT max_depth,
IdxT min_samples_split,
IdxT max_leaves,
const Dataset<DataT, LabelT, IdxT>& dataset,
Expand All @@ -359,7 +354,6 @@ void launchComputeSplitKernel(BinT* histograms,
computeSplitKernel<DataT, LabelT, IdxT, TPB_DEFAULT>
<<<grid, TPB_DEFAULT, smem_size, builder_stream>>>(histograms,
max_n_bins,
max_depth,
min_samples_split,
max_leaves,
dataset,
Expand All @@ -377,7 +371,6 @@ void launchComputeSplitKernel(BinT* histograms,
}

template void launchNodeSplitKernel<_DataT, _LabelT, _IdxT, TPB_DEFAULT>(
const _IdxT max_depth,
const _IdxT min_samples_leaf,
const _IdxT min_samples_split,
const _IdxT max_leaves,
Expand All @@ -401,7 +394,6 @@ template void launchLeafKernel<_DatasetT, _NodeT, _ObjectiveT, _DataT>(
template void launchComputeSplitKernel<_DataT, _LabelT, _IdxT, TPB_DEFAULT, _ObjectiveT, _BinT>(
_BinT* histograms,
_IdxT n_bins,
_IdxT max_depth,
_IdxT min_samples_split,
_IdxT max_leaves,
const Dataset<_DataT, _LabelT, _IdxT>& dataset,
Expand Down
8 changes: 4 additions & 4 deletions python/cuml/cuml/dask/ensemble/randomforestclassifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ class RandomForestClassifier(
* If ``False``, the whole dataset is used to build each tree.
max_samples : float (default = 1.0)
Ratio of dataset rows used while fitting each tree.
max_depth : int (default = 16)
Maximum tree depth. Must be greater than 0.
Unlimited depth (i.e, until leaves are pure)
is not supported.\n
max_depth : int or None (default = 16)
Maximum tree depth. Use ``None`` for unlimited depth (trees grow
until all leaves are pure). Must be a positive integer or ``None``.

.. note:: This default differs from scikit-learn's
random forest, which defaults to unlimited depth.
max_leaves : int (default = -1)
Expand Down
8 changes: 4 additions & 4 deletions python/cuml/cuml/dask/ensemble/randomforestregressor.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,10 @@ class RandomForestRegressor(
* If ``False``, the whole dataset is used to build each tree.
max_samples : float (default = 1.0)
Ratio of dataset rows used while fitting each tree.
max_depth : int (default = 16)
Maximum tree depth. Must be greater than 0.
Unlimited depth (i.e, until leaves are pure)
is not supported.\n
max_depth : int or None (default = 16)
Maximum tree depth. Use ``None`` for unlimited depth (trees grow
until all leaves are pure). Must be a positive integer or ``None``.

.. note:: This default differs from scikit-learn's
random forest, which defaults to unlimited depth.
max_leaves : int (default = -1)
Expand Down
18 changes: 12 additions & 6 deletions python/cuml/cuml/ensemble/randomforest_common.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,10 @@ class BaseRandomForestModel(Base, InteropMixin):
elif model.max_samples is not None:
conditional_params["max_samples"] = model.max_samples

if model.max_depth is not None:
conditional_params["max_depth"] = model.max_depth

return {
"n_estimators": model.n_estimators,
"split_criterion": split_criterion,
"max_depth": model.max_depth,
"min_samples_split": model.min_samples_split,
"min_samples_leaf": model.min_samples_leaf,
"max_features": model.max_features,
Expand Down Expand Up @@ -418,8 +416,16 @@ class BaseRandomForestModel(Base, InteropMixin):
cdef level_enum verbose = <level_enum> self._verbose_level
cdef int n_classes = self.n_classes_ if is_classifier else 0

if self.max_depth <= 0:
raise ValueError("Must specify max_depth > 0")
cdef int max_depth_c
if self.max_depth is None:
max_depth_c = np.iinfo(np.int32).max
elif not isinstance(self.max_depth, int) or self.max_depth <= 0:
raise ValueError(
f"max_depth must be a positive integer or None (unlimited); "
f"got {self.max_depth!r}"
)
else:
max_depth_c = self.max_depth

# Validate OOB score parameter
if callable(self.oob_score):
Expand Down Expand Up @@ -456,7 +462,7 @@ class BaseRandomForestModel(Base, InteropMixin):
n_bins = self.n_bins

cdef RF_params params = set_rf_params(
self.max_depth,
max_depth_c,
self.max_leaves,
max_features,
n_bins,
Expand Down
12 changes: 6 additions & 6 deletions python/cuml/cuml/ensemble/randomforestclassifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ class RandomForestClassifier(BaseRandomForestModel, ClassifierMixin):
* If ``False``, the whole dataset is used to build each tree.
max_samples : float (default = 1.0)
Ratio of dataset rows used while fitting each tree.
max_depth : int (default = 16)
Maximum tree depth. Must be greater than 0.
Unlimited depth (i.e, until leaves are pure)
is not supported.\n
.. note:: This default differs from scikit-learn's
random forest, which defaults to unlimited depth.
max_depth : int or None (default = 16)
Maximum tree depth. Use ``None`` for unlimited depth (trees grow
until all leaves are pure). Must be a positive integer or ``None``.

.. note:: This default differs from scikit-learn's random forest,
which defaults to unlimited depth.
max_leaves : int (default = -1)
Maximum leaf nodes per tree. Soft constraint. Unlimited,
If ``-1``.
Expand Down
12 changes: 6 additions & 6 deletions python/cuml/cuml/ensemble/randomforestregressor.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ class RandomForestRegressor(BaseRandomForestModel, RegressorMixin):
* If ``False``, the whole dataset is used to build each tree.
max_samples : float (default = 1.0)
Ratio of dataset rows used while fitting each tree.
max_depth : int (default = 16)
Maximum tree depth. Must be greater than 0.
Unlimited depth (i.e, until leaves are pure)
is not supported.\n
.. note:: This default differs from scikit-learn's
random forest, which defaults to unlimited depth.
max_depth : int or None (default = 16)
Maximum tree depth. Use ``None`` for unlimited depth (trees grow
until all leaves are pure). Must be a positive integer or ``None``.

.. note:: This default differs from scikit-learn's random forest,
which defaults to unlimited depth.
max_leaves : int (default = -1)
Maximum leaf nodes per tree. Soft constraint. Unlimited,
If ``-1``.
Expand Down
32 changes: 31 additions & 1 deletion python/cuml/tests/dask/test_dask_random_forest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SPDX-FileCopyrightText: Copyright (c) 2019-2025, NVIDIA CORPORATION.
# SPDX-FileCopyrightText: Copyright (c) 2019-2026, NVIDIA CORPORATION.
# SPDX-License-Identifier: Apache-2.0

import json
Expand Down Expand Up @@ -357,6 +357,36 @@ def check_count(node, nodes):
check_count(node, nodes)


def test_unlimited_max_depth_classifier(client):
n_workers = len(client.scheduler_info(n_workers=-1)["workers"])
X, y = make_classification(
n_samples=n_workers * 200, n_features=10, random_state=42
)
X = X.astype(np.float32)
y = y.astype(np.int32)

X_dask, y_dask = _prep_training_data(client, X, y, partitions_per_worker=1)
clf = cuRFC_mg(n_estimators=n_workers * 5, max_depth=None)
clf.fit(X_dask, y_dask)
preds = cp.asnumpy(cp.array(clf.predict(X_dask).compute()))
assert len(preds) == len(y)


def test_unlimited_max_depth_regressor(client):
n_workers = len(client.scheduler_info(n_workers=-1)["workers"])
X, y = make_regression(
n_samples=n_workers * 200, n_features=10, random_state=42
)
X = X.astype(np.float32)
y = y.astype(np.float32)

X_dask, y_dask = _prep_training_data(client, X, y, partitions_per_worker=1)
reg = cuRFR_mg(n_estimators=n_workers * 5, max_depth=None)
reg.fit(X_dask, y_dask)
preds = cp.asnumpy(cp.array(reg.predict(X_dask).compute()))
assert len(preds) == len(y)

Comment on lines +360 to +388

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Strengthen unlimited-depth tests to verify correctness, not just output length.

Both new tests currently pass even if predictions are wrong, because they only assert len(preds) == len(y) and predict on training data. Please compare against a scikit-learn baseline (or at minimum a quality threshold on held-out data) with fixed random_state.

Suggested test hardening
+from sklearn.ensemble import RandomForestRegressor as skrfr
@@
 def test_unlimited_max_depth_classifier(client):
@@
-    X_dask, y_dask = _prep_training_data(client, X, y, partitions_per_worker=1)
-    clf = cuRFC_mg(n_estimators=n_workers * 5, max_depth=None)
-    clf.fit(X_dask, y_dask)
-    preds = cp.asnumpy(cp.array(clf.predict(X_dask).compute()))
-    assert len(preds) == len(y)
+    X_train, X_test, y_train, y_test = train_test_split(
+        X, y, test_size=n_workers * 40, random_state=42
+    )
+    X_train_dask, y_train_dask = _prep_training_data(
+        client, X_train, y_train, partitions_per_worker=1
+    )
+    X_test_dask = from_array(X_test)
+
+    clf = cuRFC_mg(n_estimators=n_workers * 5, max_depth=None, random_state=42)
+    clf.fit(X_train_dask, y_train_dask)
+    preds = cp.asnumpy(cp.array(clf.predict(X_test_dask).compute()))
+    acc = accuracy_score(y_test, preds)
+
+    sk_clf = skrfc(
+        n_estimators=n_workers * 5, max_depth=None, random_state=42, n_jobs=-1
+    )
+    sk_clf.fit(X_train, y_train)
+    sk_acc = accuracy_score(y_test, sk_clf.predict(X_test))
+    assert acc >= (sk_acc - 0.07)
@@
 def test_unlimited_max_depth_regressor(client):
@@
-    X_dask, y_dask = _prep_training_data(client, X, y, partitions_per_worker=1)
-    reg = cuRFR_mg(n_estimators=n_workers * 5, max_depth=None)
-    reg.fit(X_dask, y_dask)
-    preds = cp.asnumpy(cp.array(reg.predict(X_dask).compute()))
-    assert len(preds) == len(y)
+    X_train, X_test, y_train, y_test = train_test_split(
+        X, y, test_size=n_workers * 40, random_state=42
+    )
+    X_train_dask, y_train_dask = _prep_training_data(
+        client, X_train, y_train, partitions_per_worker=1
+    )
+    X_test_dask = from_array(X_test)
+
+    reg = cuRFR_mg(n_estimators=n_workers * 5, max_depth=None, random_state=42)
+    reg.fit(X_train_dask, y_train_dask)
+    preds = cp.asnumpy(cp.array(reg.predict(X_test_dask).compute()))
+    r2 = r2_score(y_test, preds)
+
+    sk_reg = skrfr(
+        n_estimators=n_workers * 5, max_depth=None, random_state=42, n_jobs=-1
+    )
+    sk_reg.fit(X_train, y_train)
+    sk_r2 = r2_score(y_test, sk_reg.predict(X_test))
+    assert r2 >= (sk_r2 - 0.10)

As per coding guidelines for python/**/test_*.py: “Test files must validate numerical correctness by comparing with scikit-learn…”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@python/cuml/tests/dask/test_dask_random_forest.py` around lines 360 - 388,
The two tests test_unlimited_max_depth_classifier and
test_unlimited_max_depth_regressor only check prediction length; change them to
evaluate numerical correctness by splitting X,y into train/test via a
deterministic split (use train_test_split with a fixed random_state), use
_prep_training_data on the training portion, fit cuRFC_mg / cuRFR_mg on the
training Dask arrays, predict on the held-out test set (not the training data),
and compare predictions to a scikit-learn baseline (e.g.,
sklearn.ensemble.RandomForestClassifier/Regressor with same
n_estimators/max_depth/random_state) or assert a numeric metric (accuracy/MSE)
is within an acceptable threshold; ensure types remain float32/int32 as before
and use the same n_workers-derived sizing so tests remain distributed.


@pytest.mark.parametrize("estimator_type", ["regression", "classification"])
def test_rf_get_combined_model_right_aftter_fit(client, estimator_type):
max_depth = 3
Expand Down
33 changes: 33 additions & 0 deletions python/cuml/tests/test_random_forest.py
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,39 @@ def test_create_classification_model(
assert params["n_bins"] == verfiy_params["n_bins"]


def test_unlimited_max_depth_classifier():
X, y = make_classification(n_samples=500, n_features=10, random_state=42)

clf = curfc(n_estimators=10, max_depth=None, random_state=42)
clf.fit(X, y)
preds = clf.predict(X)
assert len(preds) == len(y)

params = clf.get_params()
assert params["max_depth"] is None
clf2 = curfc()
clf2.set_params(**params)
assert clf2.get_params()["max_depth"] is None

shallow = curfc(n_estimators=10, max_depth=2, random_state=42)
shallow.fit(X, y)
assert accuracy_score(y, preds) >= accuracy_score(y, shallow.predict(X))


def test_unlimited_max_depth_regressor():
X, y = make_regression(n_samples=500, n_features=10, random_state=42)

reg = curfr(n_estimators=10, max_depth=None, random_state=42)
reg.fit(X, y)
assert len(reg.predict(X)) == len(y)

params = reg.get_params()
assert params["max_depth"] is None
reg2 = curfr()
reg2.set_params(**params)
assert reg2.get_params()["max_depth"] is None


@pytest.mark.parametrize("n_estimators", [10, 20, 100])
@pytest.mark.parametrize("n_bins", [8, 9, 10])
def test_multiple_fits_classification(large_clf, n_estimators, n_bins):
Expand Down
20 changes: 14 additions & 6 deletions python/cuml/tests/test_sklearn_import_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,13 @@ def test_random_forest_classifier(random_state, oob_score):
n_samples=200, n_features=5, n_informative=3, random_state=random_state
)

cu_model = cuml.RandomForestClassifier(oob_score=oob_score).fit(X, y)
cu_model = cuml.RandomForestClassifier(
oob_score=oob_score,
max_depth=None,
).fit(X, y)
sk_model = sklearn.ensemble.RandomForestClassifier(
oob_score=oob_score
oob_score=oob_score,
max_depth=None,
).fit(X, y)

sk_model2 = cu_model.as_sklearn()
Expand Down Expand Up @@ -819,10 +823,14 @@ def test_random_forest_regressor(random_state, oob_score):
X, y = make_regression(n_samples=200, random_state=random_state)
X = X.astype("float32")

cu_model = cuml.RandomForestRegressor(oob_score=oob_score).fit(X, y)
sk_model = sklearn.ensemble.RandomForestRegressor(oob_score=oob_score).fit(
X, y
)
cu_model = cuml.RandomForestRegressor(
oob_score=oob_score,
max_depth=None,
).fit(X, y)
sk_model = sklearn.ensemble.RandomForestRegressor(
oob_score=oob_score,
max_depth=None,
).fit(X, y)

sk_model2 = cu_model.as_sklearn()
cu_model2 = cuml.RandomForestRegressor.from_sklearn(sk_model)
Expand Down
Loading