[mt] GPU implementation of OHE vector leaf.#12276
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the GPU hist multi-target (vector leaf) path to support categorical one-hot splits, aligning behavior with the CPU HistMultiEvaluator and adding targeted test coverage.
Changes:
- Add categorical one-hot split support to the GPU multi-target split evaluation path (scan + gain evaluation) and apply categorical splits when expanding the tree.
- Plumb feature type metadata to device evaluation kernels for categorical detection.
- Add/extend GPU Python + C++ unit tests to cover multi-target categorical one-hot behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/python-gpu/test_gpu_updaters.py | Adds a multi-target categorical OHE GPU test invocation. |
| tests/cpp/tree/gpu_hist/test_multi_evaluate_splits.cu | Adds a focused C++ GPU unit test validating categorical one-hot split selection/weights. |
| src/tree/updater_gpu_hist.cuh | Enables categorical split expansion for multi-target GPU hist (ExpandCategorical with one-hot bitfield). |
| src/tree/gpu_hist/multi_evaluate_splits.cu | Implements categorical one-hot scan + evaluation paths for multi-target GPU split search. |
| src/tree/gpu_hist/evaluate_splits.cuh | Adds WarpSum helper and threads feature type span into MultiEvaluateSplitSharedInputs. |
| src/tree/gpu_hist/evaluate_splits.cu | Switches warp reductions/broadcast masks to the shared WarpSum/WarpFullMask helpers. |
| src/common/device_helpers.cuh | Adds WarpFullMask() constant used by shuffle operations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
RAMitchell
left a comment
RAMitchell
left a comment
There was a problem hiding this comment.
LGTM, but from codex:
[P1] Scan buffer is under-sized for high-cardinality categorical features.
MakeSharedInputs passes param_.max_bin as n_bins_per_feat_tar, and EvaluateSplits() sizes scan_buffer_ as n_targets * Features() * n_bins_per_feat_tar * 2. But the new categorical scan writes using the global categorical bin_idx from feature_segments, e.g. region_others[bin_idx * n_targets + t]. Categorical cuts are not necessarily bounded by max_bin; existing tests explicitly cover n_categories = 32 with max_bin = 16 and expect categorical cut storage sized by categories, not max_bin.
Suggested fix: size the multi-target scan buffer by actual total bins per target, for example a host scalar derived from cuts_->TotalBins() / cut_values_.Size(), not Features() * max_bin. Add a regression test with multi_output_tree, categorical data, and n_categories > max_bin.
[P1] Non-OHE categorical features are silently evaluated as OHE.
The new multi-target categorical branches always run one-hot scan/evaluation and return. That diverges from existing CPU and single-target GPU behavior, which check UseOneHot(...) and otherwise use partition-based categorical splits. The added Python coverage only forces OHE for multi-target, and the partition test path is skipped when multi_target=True.
Suggested fix: either keep categorical vector-leaf GPU unsupported unless every categorical feature satisfies UseOneHot, or implement the partition path for multi-target. Add a test with multi_strategy="multi_output_tree" and max_cat_to_onehot=1 to verify the intended behavior.
Positive notes
The OHE missing-direction algebra itself looks consistent: one region stores “other categories” with missing-right, the other stores the matching bin with missing-left, and the final weight assignment uses dir to reconstruct left/right sums. The new C++ test validates the dense no-missing OHE case and expected vector weights.
|
No description provided.