[cuda] match CPU's per-group min_data_per_group semantics in categorical splits#35
Conversation
…cal splits
CPU (FeatureHistogram::FindBestThresholdCategoricalInner) treats
min_data_per_group as a minimum on the *group* -- the run of sorted
categories accumulated since the last accepted threshold. It tracks
cnt_cur_group, `continue`s past a threshold whose group is still too
small, and resets the counter to zero each time a threshold is accepted,
so acceptance depends on which earlier thresholds were accepted.
CUDA applied it as an independent per-side predicate instead:
left_count >= min_data_per_group && right_count >= min_data_per_group
That is a different rule. The two happen to agree when min_data_per_group
sits far from the per-category row counts, which is why this went
unnoticed -- but whenever it lands near them, CUDA accepts a different set
of candidate thresholds than CPU, picks a different categorical split, and
grows a different tree.
Repro (400 rows, 12 categories ~= 33 rows each, one boosting round):
min_data_per_group | 1 5 20 33 50 100 200
max|CPU - CUDA| | 0.0 0.0 0.0 5.2e-2 0.0 6.1e-2 0.0
The default is 100, so this fired on default settings.
Fix: CategoricalThresholdAcceptedByCPU() replays CPU's sequential scan for
a given threshold index and returns whether CPU would have evaluated a
split there. The scan is O(i) with i < max_cat_threshold (default 32), so
replaying it per thread is cheaper than materialising a mask in shared
memory, and it needs no extra storage: the shared-memory kernel republishes
its prefix hessians into shared_value_buffer (dead after the sort), and the
global-memory kernel already has them in hist_hess_buffer_ptr.
Applied to both categorical kernels (shared-memory and global-memory) in
both scan directions.
Adds test_cuda_min_data_per_group_categorical_matches_cpu, which asserts
bit-exact CPU/CUDA agreement across min_data_per_group in
{1, 5, 20, 33, 50, 100, 200}. Fails on the pre-fix code at 33 and 100.
Full test_dual.py suite: 126 passed, 1 skipped.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7a546c6 to
2e1cdd4
Compare
|
Rebased onto current
|
| run | result |
|---|---|
...::test_cuda_init_score_matches_cpu[5-quantile-0.5] alone |
1 passed |
...::test_cuda_init_score_matches_cpu (whole set) |
12 failed, 12 passed |
It only fails when earlier tests ran first in the same process. cudaGetLastError() returns and clears the last error from any prior CUDA call — so the warmup is surfacing a pre-existing sticky cudaErrorInvalidValue left by earlier work (most likely the quantile/percentile init-score path). Previously silent; now fatal, and attributed to the wrong place.
Two separate things worth fixing:
- The latent CUDA error itself — real, and previously being swallowed. The new check did us a favour by exposing it.
- The check's placement — clearing before checking would stop it misattributing unrelated failures (though that would re-hide [cuda] fix CUDA quantized training to match CPU #1, so I'd chase [cuda] fix CUDA quantized training to match CPU #1 first).
Why it matters for review
This is the baseline for every open PR: all 8 of my rebased branches show exactly these 12 failures and nothing else. When I say a PR has "no failures beyond master's 12", that's what I mean — none of them add a failure.
As for this PR itself (the min_data_per_group per-group semantics fix): unchanged, rebased cleanly, still 114 passed / 12 master-failures, and it remains the prerequisite for #10.
Found while rebasing the open CUDA PRs onto current
master: CUDA and CPU disagree on categorical splits at the defaultmin_data_per_group=100. This is a pre-existing bug onmaster, not something any of the open PRs introduced.The bug
CPU (
FeatureHistogram::FindBestThresholdCategoricalInner) treatsmin_data_per_groupas a minimum on the group — the run of sorted categories accumulated since the last accepted threshold. It trackscnt_cur_group, skips a threshold whose group is still too small, and resets the counter each time a threshold is accepted. Acceptance is therefore sequential: it depends on which earlier thresholds were accepted.CUDA applied it as an independent per-side predicate instead:
Different rule. The two agree when
min_data_per_groupsits far from the per-category row counts — which is why this went unnoticed — but when it lands near them, CUDA accepts a different set of candidate thresholds, picks a different categorical split, and grows a different tree.Repro
400 rows, 12 categories (~33 rows each), one boosting round:
min_data_per_groupmax|CPU − CUDA|The default is 100, so this fires on default settings.
Fix
CategoricalThresholdAcceptedByCPU()replays CPU's sequential scan for a given threshold index and returns whether CPU would have evaluated a split there. The scan is O(i) withi < max_cat_threshold(default 32), so replaying it per thread is cheaper than materialising a mask in shared memory — and it needs no extra storage:shared_value_buffer, which is dead after the sort;hist_hess_buffer_ptr.Applied to both categorical kernels, in both scan directions.
Tests
test_cuda_min_data_per_group_categorical_matches_cpuasserts bit-exact CPU/CUDA agreement acrossmin_data_per_group∈ {1, 5, 20, 33, 50, 100, 200}. It fails on the pre-fix code at 33 and 100.Validated on an RTX 5090 (CUDA 13.2): full
test_dual.py= 126 passed, 1 skipped;test_engine.pyCUDA/categorical subset green.cpplintclean with the repo's filters.Note
This unblocks #10 — its own regression test (
test_cuda_bitonic_argsort_1024_with_distinct_scores_matches_cpu) exercises the categorical path and was failing on currentmasterfor this reason, not because of anything in #10. I confirmed the failure is bit-identical (0.06108053979861565) with #10's argsort fix reverted, and vanishes entirely once this lands.🤖 Generated with Claude Code