[mt] Split up the tree model weights, support extmem.#11814
[mt] Split up the tree model weights, support extmem.#11814trivialfis merged 13 commits intodmlc:masterfrom
Conversation
Device copy. cleanup, mapping. IO. view. split targets. Method. small cleanups. set leaves. Fix cpp tests. Test set leaves. more specific. derive the size. check. check. check.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the multi-target tree model to separate internal base weights from leaf output weights. The key motivation is that base weights can be smaller when using reduced gradients, while leaf weights always need to be full-size for predictions.
Key changes:
- Split
weights_intoweights_(internal base weights) andleaf_weights_(output weights) - Reuse the
right_child field to store leaf weight indices for leaf nodes - Add
SetLeaves()method to copy base weights to leaf weights for non-reduced trees
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| include/xgboost/multi_target_tree_model.h | Add leaf_weights_ member, split NodeWeight methods, add SetLeaves() and NumSplitTargets() |
| src/tree/multi_target_tree_model.cc | Implement weight splitting logic, update serialization to save both weight types |
| src/tree/tree_view.h | Rename weights to leaf_weights, update LeafValue() to use leaf index mapping |
| src/tree/tree_view.cc | Simplify view constructors, update to use LeafWeights() |
| src/tree/io_utils.h | Add kLeafWeight field for serialization |
| src/tree/updater_quantile_hist.cc | Add ExpandTreeLeaf() calls to copy base weights to leaves |
| src/tree/updater_gpu_hist.cuh | Rename UpdateTreeLeaf to ExpandTreeLeaf, add SetLeaves() call |
| tests/cpp/tree/test_multi_target_tree_model.cc | Add comprehensive tests for new SetLeaves() functionality |
| tests/cpp/predictor/test_predictor.cc | Update test to call SetLeaves() |
| src/common/cuda_stream.h | Add CUDA guards for non-CUDA builds |
| src/common/cuda_rt_utils.h/.cc | Add MemcpyAsync utility function |
| src/gbm/gbtree.h | Fix error message notation from (0, to [0, |
| src/tree/tree_model.cc | Add early return for root node in GetDepth() |
Comments suppressed due to low confidence (1)
src/tree/multi_target_tree_model.cc:354
- The
MemCostBytes()function is missingleaf_weights_.SizeBytes()in its calculation. With the introduction of the separateleaf_weights_member, it should be included in the memory cost calculation.
[[nodiscard]] std::size_t MultiTargetTree::MemCostBytes() const {
std::size_t n_bytes = 0;
n_bytes += left_.SizeBytes();
n_bytes += right_.SizeBytes();
n_bytes += parent_.SizeBytes();
n_bytes += split_index_.SizeBytes();
n_bytes += default_left_.SizeBytes();
n_bytes += split_conds_.SizeBytes();
n_bytes += weights_.SizeBytes();
return n_bytes;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
cc @rongou |
Ref #9043 .