-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
[mt] Cleanup partitioners. #11820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[mt] Cleanup partitioners. #11820
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the GPU histogram tree builder code by introducing a RowPartitionerBatches wrapper struct to consolidate partitioner management logic and improves code clarity by renaming node_sum to child_sum in split candidate structures.
Key changes:
- Introduces
RowPartitionerBatchesstruct to encapsulate partitioner vector operations with a cleaner API - Renames
node_sumtochild_sumthroughout multi-target split code for better semantic clarity - Removes dead code (unused
sum_zeroscalculation) and redundant checks
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tree/gpu_hist/row_partitioner.cuh | Adds new RowPartitionerBatches struct with Init method and accessor functions to wrap partitioner vector management |
| src/tree/updater_gpu_hist.cuh | Replaces std::vector<std::unique_ptr<RowPartitioner>> with RowPartitionerBatches type and updates method calls to use new API (At, Size, Empty) |
| src/tree/updater_gpu_hist.cu | Similar updates as .cuh file, replacing vector operations with RowPartitionerBatches methods |
| src/tree/updater_gpu_common.cuh | Renames node_sum member to child_sum in MultiSplitCandidate struct |
| src/tree/gpu_hist/multi_evaluate_splits.cu | Updates references from node_sum to child_sum and removes unused sum_zeros calculation |
| src/tree/gpu_hist/expand_entry.cu | Updates output stream operator to use child_sum instead of node_sum |
| tests/cpp/tree/gpu_hist/test_multi_evaluate_splits.cu | Updates test assertions to check child_sum instead of node_sum |
| python-package/xgboost/testing/multi_target.py | Adds min_cache_page_bytes parameter to test iterator configuration for better external memory testing |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 12 out of 12 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