Fix RandomForestClassifier sklearn child metadata

[csadorf]

Fix RandomForestClassifier.as_sklearn() so exported child estimators use sklearn-compatible encoded classes_ and n_classes_ metadata, while the exported forest keeps the original labels.

Adds coverage with non-contiguous classifier labels and compares the exported child estimator metadata against a fitted sklearn random forest.

Contributes to #8180.
Related to #8238.

[coderabbitai]

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: 982059e5-2719-4e51-a022-b12cc40acbe3

📥 Commits

Reviewing files that changed from the base of the PR and between 6ba691f and b1323b7.

📒 Files selected for processing (2)

• python/cuml/cuml/ensemble/randomforestclassifier.py
• python/cuml/tests/test_sklearn_import_export.py

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved scikit-learn export for random forest classifiers so the converted model now keeps class metadata consistent at both the model and estimator level.
  • Fixed per-tree class information to better match expected scikit-learn behavior after conversion.
• Tests
  • Added and expanded checks to verify exported random forest classifiers preserve class labels and class counts correctly.

Walkthrough

The PR updates cuML RandomForestClassifier CPU export so estimator-level class metadata is rewritten consistently, and extends sklearn import/export tests to verify top-level and per-estimator class attributes during conversion.

Changes

RandomForestClassifier class metadata

Layer / File(s)	Summary
CPU export class metadata python/cuml/cuml/ensemble/randomforestclassifier.py	RandomForestClassifier._attrs_to_cpu now post-processes exported attrs to update each estimator’s classes_ and n_classes_, and returns matching top-level classes_/n_classes_.
sklearn export tests python/cuml/tests/test_sklearn_import_export.py	The existing round-trip test now checks each exported estimator’s class metadata, and a new as_sklearn() test compares top-level and per-estimator class attributes against scikit-learn.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 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%.	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 summarizes the main change: fixing sklearn child metadata for RandomForestClassifier.
Description check	✅ Passed	The description matches the code changes and tests around sklearn-compatible child metadata and exported labels.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[jcrist]

Nice fix. One question, but otherwise LGTM.

[jcrist]

Should this be handled by treelite.sklearn.export_model instead of here?

[csadorf]

Yes, perhaps, this is a bit of hack. Maybe we can pursue a more comprehensive solution as part of #8238 .

[jcrist]

I think hacking it in here now is fine, but IMO this should be upstreamed to treelite (and eventually removed here).

[csadorf]

@hcho3 Can we track this as a feature request for treelite?

[csadorf]

@coderabbitai please create a follow-up issue for this.

[coderabbitai]

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search likely Treelite repositories =="
gh search repos treelite --limit 10 --json nameWithOwner,description,url | jq -r '.[] | "\(.nameWithOwner)\t\(.url)\t\(.description)"'

echo
echo "== Search for existing related issues in likely upstream repo candidates =="
for repo in dmlc/treelite treelite/treelite rapidsai/cuml; do
  echo "-- $repo --"
  gh issue list --repo "$repo" --state all --limit 20 --search 'export_model sklearn child estimator classes n_classes_ classes_ random forest' || true
  echo
done

echo "== If dmlc/treelite exists, show basic repo info =="
gh repo view dmlc/treelite --json nameWithOwner,description,url,defaultBranchRef || true

Length of output: 1460

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search likely Treelite repositories =="
gh search repos treelite --limit 10 --json fullName,description,url | \
  jq -r '.[] | "\(.fullName)\t\(.url)\t\(.description // "")"'

echo
echo "== View canonical upstream repo if available =="
gh repo view dmlc/treelite --json name,owner,description,url,defaultBranchRef || true

echo
echo "== Search for existing related issues in dmlc/treelite =="
gh issue list --repo dmlc/treelite --state all --limit 50 \
  --search 'sklearn export_model child estimator classes n_classes_ RandomForestClassifier' || true

echo
echo "== Search more broadly for similar issues in dmlc/treelite =="
gh search issues --repo dmlc/treelite \
  'treelite sklearn export_model estimator classes n_classes_' \
  --limit 20 || true

echo
echo "== Create follow-up issue in dmlc/treelite =="
tmp_body="$(mktemp)"
cat > "$tmp_body" <<'EOF'
## Summary

`treelite.sklearn.export_model` should export scikit-learn-compatible classifier metadata for child estimators in exported random forest classifiers.

Today, cuML needs a local workaround when converting a `RandomForestClassifier` to scikit-learn: after calling the CPU export path, it manually patches each exported child estimator so that:

- the forest-level `classes_` retains the original class labels
- each child estimator's `classes_` uses sklearn-style encoded class indices (`[0, 1, ..., n_classes - 1]`)
- each child estimator's `n_classes_` matches the parent classifier

This behavior is currently implemented downstream in RAPIDS cuML, but it appears to be a responsibility that belongs in Treelite's sklearn export layer.

## Rationale

scikit-learn stores classifier metadata differently at the forest level vs. the individual child trees. If `treelite.sklearn.export_model` exports the same `classes_` metadata everywhere, downstream projects need ad hoc fixups to restore sklearn-compatible child estimator state.

Moving this logic upstream would:

- keep sklearn-export behavior consistent at the source
- remove downstream patching/hacks in RAPIDS cuML
- improve compatibility for any Treelite consumer exporting sklearn random forest classifiers

## Affected area

- `treelite.sklearn.export_model`
- sklearn random forest classifier export path
- exported child estimator metadata (`classes_`, `n_classes_`)

## Observed downstream workaround

RAPIDS cuML PR `#8291` adds a post-processing step in `python/cuml/cuml/ensemble/randomforestclassifier.py` to patch exported child estimators after CPU export.

PR: https://github.com/rapidsai/cuml/pull/8291  
Review comment: https://github.com/rapidsai/cuml/pull/8291#discussion_r3470275711

Requested by: `@csadorf`

## Expected behavior / acceptance criteria

- Exported sklearn random forest classifiers preserve the original labels at the forest level (`forest.classes_`)
- Exported child estimators use sklearn-compatible encoded labels for `estimator.classes_`
- Exported child estimators expose the correct `estimator.n_classes_`
- Behavior matches a natively fitted scikit-learn `RandomForestClassifier`, including cases with non-contiguous class labels
- Downstream projects such as cuML can remove local metadata patching once they adopt the updated Treelite behavior
EOF

issue_url="$(gh issue create \
  --repo dmlc/treelite \
  --title 'treelite.sklearn.export_model should preserve sklearn-compatible child estimator class metadata for random forest classifiers' \
  --body-file "$tmp_body" \
  --assignee csadorf 2>/tmp/gh_issue_create_err || true)"

if [[ -n "${issue_url}" ]]; then
  echo "CREATED_ISSUE_URL=${issue_url}"
else
  echo "Issue creation with assignee failed, retrying without assignee..." >&2
  cat /tmp/gh_issue_create_err >&2 || true
  issue_url="$(gh issue create \
    --repo dmlc/treelite \
    --title 'treelite.sklearn.export_model should preserve sklearn-compatible child estimator class metadata for random forest classifiers' \
    --body-file "$tmp_body")"
  echo "CREATED_ISSUE_URL=${issue_url}"
fi

Length of output: 6060

---

---

@csadorf Created the follow-up upstream issue in dmlc/treelite: #664
dmlc/treelite#664

I also checked for an existing matching issue first and didn’t find one. I wasn’t able to assign it via the integration because GitHub returned a permissions error (Resource not accessible by integration).

---

🧠 Learnings used

Learnt from: csadorf
Repo: rapidsai/cuml PR: 7593
File: python/cuml/cuml/benchmark/algorithms.py:2-2
Timestamp: 2026-02-25T22:16:12.641Z
Learning: In the rapidsai/cuml repository, copyright headers in Python source files are maintained automatically. Do not review or comment on copyright header year ranges for Python sources.

---

Learnt from: jcrist
Repo: rapidsai/cuml PR: 7957
File: python/cuml/tests/test_linear_regression.py:64-66
Timestamp: 2026-04-07T22:06:24.687Z
Learning: When reviewing Python code in rapidsai/cuml, avoid flagging or requesting fixes for one-off Ruff linter warnings (e.g., RUF059: unused unpacked variables) unless that specific Ruff rule is already enabled/enforced project-wide via the repository’s Ruff configuration. Maintain a consistent all-or-nothing policy: only raise issues for Ruff rules that CI/Ruff is enforcing; don’t require incremental, PR-scoped cleanups for unenforced rules.

---

Learnt from: csadorf
Repo: rapidsai/cuml PR: 7960
File: python/cuml/cuml_accel_tests/upstream/scikit-learn/example_collector.py:70-73
Timestamp: 2026-04-08T21:32:13.634Z
Learning: In the rapidsai/cuml repository, do not request fixes or flag Ruff rule B904 (“Within an except clause, raise exceptions with raise ... from err or raise ... from None”) in code review unless Ruff/CI is explicitly enforcing B904 across the repository. This exception is intentional; if B904 is not part of the enforced rule set (e.g., via CI Ruff configuration), treat its violations as acceptable and avoid recommending the “raise ... from err/None” change.

---

Learnt from: jcrist
Repo: rapidsai/cuml PR: 8043
File: python/cuml/cuml/explainer/sampling.py:0-0
Timestamp: 2026-05-01T18:50:52.183Z
Learning: In rapidsai/cuml code, when a function is decorated with `cuml.internals.reflect` (e.g., explainer helpers like `kmeans_sampling`), rely on the decorator to coerce any returned output arrays to match the memory type of the input `X`. In that case, avoid redundantly tracking or propagating `output_type`/dtype strings (e.g., `_output_dtype_str`) for intermediate or final steps inside the decorated function. If returning values, prefer wrapping return arrays in `CumlArray` so the decorator has clear, explicit array semantics to reflect.

---

Learnt from: csadorf
Repo: rapidsai/cuml PR: 8034
File: .agents/test-cuml/SKILL.md:97-111
Timestamp: 2026-05-01T20:45:11.614Z
Learning: In the rapidsai/cuml repository, “UCXX” refers to the RAPIDS UCX communication library (https://github.com/rapidsai/ucxx). It is distinct from “UCX” and should not be treated as a misspelling of “UCX”. Do not flag or suggest replacing “UCXX” with “UCX” anywhere in cuML documentation or source code.

---

Learnt from: jcrist
Repo: rapidsai/cuml PR: 8051
File: python/cuml/cuml/naive_bayes/naive_bayes.py:621-627
Timestamp: 2026-05-06T02:07:12.476Z
Learning: When reviewing Python estimator code in rapidsai/cuml, if a public parameter is being fully removed, it is acceptable and intentional to set the default in the public signature to a sentinel like 'deprecated' (e.g., def fit(..., sample_weight='deprecated')). Do not flag or require reverting to None to match upstream libraries like scikit-learn; the mismatch is deliberate because the parameter is being removed entirely. This pattern should be applied to parameters being removed across Python estimator code, with project-level documentation and tests updated to reflect the sentinel usage and removal timeline.

---

Learnt from: hcho3
Repo: rapidsai/cuml PR: 8048
File: python/cuml/cuml/ensemble/randomforestclassifier.py:284-305
Timestamp: 2026-05-09T09:24:07.532Z
Learning: When reviewing cuML ensemble code that integrates NVForest (rapidsai/nvforest), do not flag calls to `nvforest_model.forest.get_dtype()` as an invalid/non-existent API. `nvforest_model.forest` is the internal forest object exposed via the `ForestInference` returned by `as_nvforest()`, and its `get_dtype()` method is expected to exist and return `np.float32` or `np.float64` depending on whether the model is configured for double precision.

[csadorf]

/merge