[cuda] implement cost-effective gradient boosting on the CUDA tree learner#18
[cuda] implement cost-effective gradient boosting on the CUDA tree learner#18maxwbuckley wants to merge 1 commit into
Conversation
|
Thank you, Max — and thank you, Claude Code (independently 🙂). Verdict: SOLID, no blocking bug for the supported/tested usage. Reviewed against
Two non-blocking notes (uncommon paths, worth a docstring or follow-up, not a merge blocker):
Your documented "retroactive re-eval" limitation is real and honestly stated — the tests just don't engineer a case that triggers a promotion. Fine to leave as-is. Merge blocker is only the required P.S. — you proved bit-for-bit leaf-count and feature-set parity across 6 CEGB configs and then shipped unformatted test code. 😄 The penalty for skipping |
|
Thank you, Max — and thank you, Claude Code (independently 🙂). Rebase nudge 👉 This one reviewed out SOLID — it's only CONFLICTING because You have write access now (accept the repo invite if you haven't), so once it's rebased and green you can merge it yourself. 🚀 |
831c398 to
4919248
Compare
|
Rebased onto current Please re-review — there is one behavioural change I had to make, called out below. What changed. Rebuilt as
The original PR called it inline in CEGB charges This is a change from what you reviewed, and it's the bit I'd most like a second opinion on. Also: the conflict in In Validation (RTX 5090, CUDA 13.2, built from source): 7 CEGB tests pass (including the lazy-penalty rejection); full Rebased, re-integrated and validated with Claude Code; real GPU runs, not CI. |
4919248 to
27fca03
Compare
…arner Implements CEGB (cegb_tradeoff, cegb_penalty_split, cegb_penalty_feature_coupled) in the CUDA best-split finder, matching the CPU CostEfficientGradientBoosting behavior: - the per-split cost delta (tradeoff * penalty_split * num_data_in_leaf, plus tradeoff * coupled[real_fidx] while a feature is unused in the model) is subtracted from each task's finalized gain in the dispatch kernels, before cross-feature / cross-leaf comparison - exactly where the CPU serial learner applies new_split.gain -= cegb_->DeltaGain(...) - per-task coupled penalties live in a device array; the host tracks which features have been used in any split (mirroring is_feature_used_in_split_, accumulated over the whole model) and zeroes a feature's penalty entries the first time it is used - FindBestFromAllSplitsKernel now requires gain > 0 to select a split, mirroring the CPU stop condition (best gain <= 0 -> stop). Without CEGB this is a no-op since valid splits always have positive gain; with CEGB the penalty can push all gains negative and growth must stop - cegb_penalty_feature_lazy (per-row-per-feature cost tracking) is not implemented and now fails fast with a clear error This replaces the previous blanket Log::Fatal guard: split and coupled penalties now work on CUDA; only the lazy penalty is still rejected. Before this change CUDA silently ignored CEGB: with cegb_penalty_split=1.0 CPU stopped at a single root leaf while CUDA grew 31 leaves in every tree (prediction divergence 0.91). After this change per-tree leaf counts and selected features match CPU exactly and predictions agree at FP epsilon (<= 2.2e-16) on all test configurations. Adds parametrized parity tests (6 CEGB configs) plus a lazy-penalty rejection test to test_dual.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> (cherry picked from commit 04a3cf7)
27fca03 to
42fbe46
Compare
|
Re-rebased onto current CEGB is incompatible with hybrid level-batched growthValidating on the new Root cause is structural, not a plumbing bug. CEGB is order-dependent: Fix: fall back to the classic leaf-wise loop when CEGB is enabled, exactly the way the batched path is already gated off for categorical features: if (CostEfficientGradientBoosting::IsEnable(config_)) {
use_hybrid_growth_ = false;
}Confirmed empirically: Worth knowing independently of this PR: any order-dependent feature is unsafe on the batched path. I gated monotone constraints off the same way in #16. Also
ValidationRTX 5090, CUDA 13.2: 7 CEGB tests pass; Validated with Claude Code on real GPU runs; CI has no GPU and never exercises any of this. |
Problem: the delta
When training with
device_type="cuda", the cost-effective gradient boosting parameters (cegb_tradeoff,cegb_penalty_split,cegb_penalty_feature_coupled,cegb_penalty_feature_lazy) were silently ignored. CEGB lives only in the CPU serial learner (CostEfficientGradientBoosting), which subtracts a per-split cost from each candidate's gain; the CUDA kernels had no penalty plumbing, and the CUDA learner had no "stop when best penalized gain ≤ 0" condition.Measured delta before this PR (400×6 synthetic data,
learning_rate=0.1, 5 rounds,gpu_use_dp=true):cegb_penalty_split=0.1cegb_penalty_split=1.0cegb_penalty_split=5.0cegb_tradeoff=0.5, penalty_split=1.0CPU prunes aggressively (down to a stump when the penalty exceeds all gains); CUDA grew full 31-leaf trees regardless — the penalties had zero effect.
Fix
Implements CEGB in the CUDA best-split finder:
tradeoff · penalty_split · num_data_in_leaf + tradeoff · coupled[real_fidx](coupled term only while a feature is unused anywhere in the model) is subtracted from each task's finalized gain in the dispatch kernels — before cross-feature/cross-leaf comparison, exactly where CPU appliesnew_split.gain -= cegb_->DeltaGain(...).is_feature_used_in_split_(accumulated over the whole model, not per tree) and zeroes a feature's penalty entries the first time it's used in any split.FindBestFromAllSplitsKernelnow requiresgain > 0to select a split, mirroring CPU'sif (best_leaf_SplitInfo.gain <= 0.0) break. Without CEGB this is a no-op (valid splits always have positive gain); with CEGB the penalty can push every gain negative and growth must stop — this was the missing piece that let CUDA keep splitting at gain ≤ 0.cegb_penalty_feature_lazy: needs per-(row, feature) cost tracking (a device bitset + per-leaf scan); not implemented — now rejected with a clearLog::Fatalinstead of being silently ignored.The first commit's blanket guard is replaced: split and coupled penalties work on CUDA; only the lazy penalty is still rejected.
Result: the delta after the fix
Same measurement, post-fix:
cegb_penalty_split=0.1cegb_penalty_split=1.0cegb_penalty_split=5.0coupled=[5,5,5,5,5,5]coupled=[0.1, 0.1, 5, 5, 5, 5]cegb_tradeoff=0.5, penalty_split=1.0Identical tree shapes and feature selection on every case; predictions match bit-for-bit (≤ 1 fp64 ULP).
Tests (in
test_dual.py, gated onTASK=cuda)test_cuda_cegb_matches_cpu— 6 parametrized CEGB configs, asserting per-tree leaf counts, feature sets, and predictions (atol=1e-10) all match CPU. All fail on the unpatched build (CUDA grows 31-leaf trees); all pass with the fix.test_cuda_cegb_lazy_penalty_raises— the unimplemented lazy penalty raises a clear error.Full
test_dual.pysuite (61 tests) passes.Known limitation
CPU's
UpdateLeafBestSplitsretroactively re-evaluates cached splits in other leaves when a feature first becomes used (the coupled penalty disappears for those cached candidates). CUDA caches one best split per leaf and cannot recover a runner-up without a per-(leaf, feature) buffer; in adversarial coupled-penalty configurations this second-order effect could pick a different (still valid) split order. Not observed in any test configuration; documented for completeness.🤖 Generated with Claude Code