Replace RF global quantile sort with deterministic sampled quantiles

[RAMitchell]

This PR replaces Random Forest's per-feature full-column quantile sort with deterministic uniform sampled quantiles. This prepares the RF training path for distributed row-global training, where global sorting is not viable.

Refs #7969.

Changes

• Add deterministic per-feature quantile sampling using the existing RF hash utility.
• Keep exact behavior when the sample count covers all rows.
• Remove lower/upper sentinel quantiles.
• Rely on lower_bound clamping for values outside the quantile range.
• Update RF quantile tests for sampled bins, determinism, exact fallback, and sentinel-free behavior.

Distributed Direction

The intended distributed extension is to assign a fixed global sample budget per feature across workers, have each worker fill its assigned sample slots from local rows, then combine those samples so every worker builds identical quantile bins. A later histogram all-reduce can then make workers build identical trees.

Validation

• Current RF suite: 200/200 passed.
• Quantile-focused tests: 51/51 passed.
• Baseline exact-quantile RF property/accuracy suite: 100/100 passed.
• Current sampled-quantile RF property/accuracy suite: 100/100 passed.

Accuracy

Held-out C++ accuracy experiments comparing baseline exact quantiles vs current sampled quantiles:

Case	Baseline	Current
25k classification, normal order	1.000000 acc	1.000000 acc
25k classification, label-sorted	1.000000 acc	1.000000 acc
25k regression, normal order	0.566670 R2	0.566935 R2
25k regression, target-sorted	0.567602 R2	0.571527 R2
100k classification, normal order	1.000000 acc	1.000000 acc
100k classification, label-sorted	1.000000 acc	1.000000 acc
100k regression, normal order	0.422733 R2	0.422221 R2
100k regression, target-sorted	0.422830 R2	0.423043 R2

Accuracy is unchanged for classification and neutral for regression.

Performance

Case	Baseline	Current
RFClassifier<float>/blobs/0	2018.6 ms	2029.2 ms
RFClassifier<float>/blobs/1	2379.8 ms	2341.1 ms
RFClassifier<double>/blobs/0	2265.9 ms	2218.1 ms

Performance is broadly neutral.

[copy-pr-bot]

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.

[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: 271faa0b-fa61-452d-9662-4f84c54d3bb5

📥 Commits

Reviewing files that changed from the base of the PR and between 3da99d3 and 0ad7698.

📒 Files selected for processing (2)

• cpp/src/decisiontree/batched-levelalgo/quantiles.cuh
• cpp/tests/sg/rf_test.cu

🚧 Files skipped from review as they are similar to previous changes (1)

• cpp/src/decisiontree/batched-levelalgo/quantiles.cuh

---

📝 Walkthrough

Summary by CodeRabbit

• New Features

  • Quantile API accepts an oversampling factor and an explicit seed for reproducible sampling.

• Improvements

  • Quantile/bin computation can sample uniformly instead of sorting full columns for faster, lower-memory operation.
  • Deterministic quantiles when the same seed is used.

• Bug Fixes

  • Lower-bound now clamps values beyond the last quantile to the final bin.

• Tests

  • Added/updated tests for sampled-quantile determinism, exact-sample fallback, and rank-error bounds.

Walkthrough

Extracts FNV1a hashing to a shared header, implements seeded uniform per-column sampling for quantile computation (new kernel and API), updates RandomForest to pass oversampling and seed, and adjusts/extends tests to validate sampling behavior and determinism.

Changes

Sampled quantile computation and utilities

Layer / File(s)	Summary
FNV1a hashing utility extraction cpp/src/decisiontree/batched-levelalgo/random_utils.cuh	New shared header defining FNV1a prime/basis constants and fnv1a32 hash function.
Update builder kernels to use shared hashing cpp/src/decisiontree/batched-levelalgo/kernels/builder_kernels.cuh	Include the new random_utils.cuh and remove local FNV1a definitions; update lower_bound documentation.
Sampled quantile computation with kernel cpp/src/decisiontree/batched-levelalgo/quantiles.cuh	Rewrite computeQuantiles to sample uniformly per column using gatherUniformSampledColumnKernel, add oversampling_factor and seed parameters, allocate sampled/sorted temporaries, radix-sort samples, and compute quantiles from sampled data.
Random Forest quantile call update cpp/src/randomforest/randomforest.cuh	Update RandomForest::fit to call DT::computeQuantiles with oversampling_factor=4 and rf_params.seed.
Test updates and new sampling validation cpp/tests/sg/rf_test.cu	Add missing includes, assert lower_bound clamps above-last-bin, replace histogram-based checks with unique-bin bounds and monotonicity assertions, update variable-bin invocation to pass seed/full-sample, and add sampled-quantile exact-fallback, determinism, and rank-error tests (float/double).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 6.67% 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 'Replace RF global quantile sort with deterministic sampled quantiles' directly and accurately summarizes the main change: replacing exact quantile sorting with sampled quantiles.
Description check	✅ Passed	The description comprehensively explains the changeset, covering objectives, implementation approach, validation results, accuracy and performance impacts, and future distributed direction.
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 and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

cpp/src/decisiontree/batched-levelalgo/quantiles.cuh (1)

91-108: ⚡ Quick win

Add null pointer validation for data parameter.

The function validates max_n_bins, n_rows, n_cols, and oversampling_factor but does not check if data is null. Dereferencing a null pointer in the sampling kernel or cudaMemcpyAsync would cause undefined behavior.

🛡️ Proposed fix

 {
   raft::common::nvtx::push_range("computeQuantiles");
+  RAFT_EXPECTS(data != nullptr, "data pointer must not be null");
   RAFT_EXPECTS(max_n_bins > 0, "max_n_bins must be positive");
   RAFT_EXPECTS(n_rows > 0, "n_rows must be positive");

As per coding guidelines: "Implement proper input validation for negative dimensions and null pointers".

🤖 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 `@cpp/src/decisiontree/batched-levelalgo/quantiles.cuh` around lines 91 - 108,
computeQuantiles currently validates sizes but not the input pointer; add a
null-pointer check for the data parameter at the top of computeQuantiles (before
any use of data, sampling kernels or cudaMemcpyAsync) and fail fast using the
same RAFT_EXPECTS style used for other inputs (e.g., RAFT_EXPECTS(data !=
nullptr, "data pointer must not be null")). This ensures any dereference of data
in functions like computeQuantiles or downstream sampling logic is protected.

🤖 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.

Nitpick comments:
In `@cpp/src/decisiontree/batched-levelalgo/quantiles.cuh`:
- Around line 91-108: computeQuantiles currently validates sizes but not the
input pointer; add a null-pointer check for the data parameter at the top of
computeQuantiles (before any use of data, sampling kernels or cudaMemcpyAsync)
and fail fast using the same RAFT_EXPECTS style used for other inputs (e.g.,
RAFT_EXPECTS(data != nullptr, "data pointer must not be null")). This ensures
any dereference of data in functions like computeQuantiles or downstream
sampling logic is protected.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d83acae8-cdaf-4e79-930e-1d79f3b880ae

📥 Commits

Reviewing files that changed from the base of the PR and between 22775f8 and 3da99d3.

📒 Files selected for processing (2)

• cpp/src/decisiontree/batched-levelalgo/quantiles.cuh
• cpp/tests/sg/rf_test.cu

[dantegd]

Very nice PR, just had a few (very) minor questions and suggestions.

[RAMitchell]

@dantegd thanks can you merge?

[dantegd]

/merge

[RAMitchell]

/merge