Skip to content

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Oct 20, 2025

  • Extract the device model view and share it with CPU.
  • Ensure the model is locked when pulling tree data.
  • Use host device vector for storing the tree group.

This PR eliminates another data copy (tree group), and ensures thread safety when the model is trained using GPU but inference is performed with CPU.

@trivialfis trivialfis requested a review from Copilot October 20, 2025 19:00
Copy link
Contributor

@Copilot Copilot AI left a 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 tree model view infrastructure to enable sharing between CPU and GPU predictors, improving code maintainability and reducing duplication.

Key Changes:

  • Introduced GBTreeModelView template class to unify model view creation across CPU and GPU
  • Changed tree view constructors to accept DeviceOrd instead of Context*
  • Converted tree_info from std::vector<int> to HostDeviceVector<bst_target_t> for device-agnostic access

Reviewed Changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/predictor/gbtree_view.h New template class for unified tree model views
src/tree/tree_view.h Updated constructors to accept DeviceOrd instead of Context*
src/tree/tree_view.cc Refactored to use DeviceOrd for device dispatch
src/predictor/gpu_predictor.cu Replaced DeviceModel with GBTreeModelView template
src/predictor/cpu_predictor.cc Adopted GBTreeModelView and unified leaf index logic
src/gbm/gbtree_model.h Changed tree_info to HostDeviceVector and added TreeGroups() method
src/gbm/gbtree_model.cc Updated to use HostDeviceVector for tree_info
tests/cpp/predictor/test_predictor.h Updated test helper to return unique_ptr
tests/cpp/predictor/test_predictor.cc Updated tests to use new model view interface
tests/cpp/predictor/test_gpu_predictor.cu Updated GPU tests for new interface
tests/cpp/predictor/test_cpu_predictor.cc Updated tree view constructor call
src/gbm/gbtree.cc Moved InitTreesToUpdate implementation and updated tree_info access
src/common/device_vector.cuh Fixed resize implementation for CTK12.8 compatibility
plugin/sycl/predictor/predictor.cc Updated to access tree_info through HostDeviceVector
src/data/cat_container.h Forward declared Json instead of including header
tests/cpp/gbm/test_gbtree.cc Added clarifying comment

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@trivialfis trivialfis marked this pull request as ready for review October 20, 2025 19:09
@trivialfis trivialfis changed the title [wip] Share the model view for CPU and GPU. Share the model view for CPU and GPU. Oct 20, 2025
@trivialfis
Copy link
Member Author

cc @rongou

@trivialfis trivialfis mentioned this pull request Oct 20, 2025
3 tasks
@trivialfis trivialfis merged commit 3868b5f into dmlc:master Oct 21, 2025
82 of 85 checks passed
@trivialfis trivialfis deleted the persist-model-1 branch October 21, 2025 11:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants