Skip to content

[train] Fold v2.XGBoostTrainer API into the public trainer class as an alternate constructor#50045

Merged
justinvyu merged 17 commits intoray-project:masterfrom
justinvyu:xgboost/merge_v2
Mar 11, 2025
Merged

[train] Fold v2.XGBoostTrainer API into the public trainer class as an alternate constructor#50045
justinvyu merged 17 commits intoray-project:masterfrom
justinvyu:xgboost/merge_v2

Conversation

@justinvyu
Copy link
Copy Markdown
Contributor

Summary

Currently, the new XGBoostTrainer API is only accessible with a separate import ray.train.xgboost.v2.XGBoostTrainer.

To avoid unnecessary import changes, this PR folds the new API, which accepts new arguments (train_loop_per_worker, train_loop_config, xgboost_config), into the public ray.train.xgboost.XGBoostTrainer class.

This also makes some changes in the Ray Train v2 XGBoostTrainer class to improve the migration UX, since it does not support the legacy XGBoostTrainer API at all.

TODO

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Copy link
Copy Markdown
Contributor

@matthewdeng matthewdeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very elegant!

Comment on lines +130 to +133
# TODO(justinvyu): [Deprecated] Legacy XGBoostTrainer API
label_column: Optional[str] = None,
params: Optional[Dict[str, Any]] = None,
num_boost_round: Optional[int] = None,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these needed for the V2 API? These are not passed in from the V1 API.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is train/v2/xgboost, not train/xgboost/v2, so people who use RAY_TRAIN_V2_ENABLED would possibly be passing in the xgboost V1 API. 😂 😭 💀 🪦

Copy link
Copy Markdown
Contributor

@hongpeng-guo hongpeng-guo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Comment on lines +130 to +133
# TODO(justinvyu): [Deprecated] Legacy XGBoostTrainer API
label_column: Optional[str] = None,
params: Optional[Dict[str, Any]] = None,
num_boost_round: Optional[int] = None,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is train/v2/xgboost, not train/xgboost/v2, so people who use RAY_TRAIN_V2_ENABLED would possibly be passing in the xgboost V1 API. 😂 😭 💀 🪦

Comment on lines +191 to +205
train_loop_per_worker: Optional[
Union[Callable[[], None], Callable[[Dict], None]]
] = None,
train_loop_config: Optional[Dict] = None,
xgboost_config: Optional[XGBoostConfig] = None,
scaling_config: Optional[ray.train.ScalingConfig] = None,
run_config: Optional[ray.train.RunConfig] = None,
datasets: Optional[Dict[str, GenDataset]] = None,
dataset_config: Optional[ray.train.DataConfig] = None,
resume_from_checkpoint: Optional[Checkpoint] = None,
metadata: Optional[Dict[str, Any]] = None,
# TODO(justinvyu): [Deprecated] Legacy XGBoostTrainer API
label_column: Optional[str] = None,
params: Optional[Dict[str, Any]] = None,
num_boost_round: Optional[int] = None,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: it's fine to change the ordering of these params because they were already forced to be kwargs.

…ost/merge_v2

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Copy link
Copy Markdown
Contributor

@matthewdeng matthewdeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat!

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu justinvyu enabled auto-merge (squash) March 11, 2025 06:35
@github-actions github-actions bot added the go add ONLY when ready to merge, run all tests label Mar 11, 2025
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@github-actions github-actions bot disabled auto-merge March 11, 2025 16:04
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu justinvyu merged commit 9247aff into ray-project:master Mar 11, 2025
5 checks passed
@justinvyu justinvyu deleted the xgboost/merge_v2 branch March 11, 2025 18:09
qinyiyan pushed a commit to qinyiyan/ray that referenced this pull request Mar 13, 2025
… an alternate constructor (ray-project#50045)

Currently, the new `XGBoostTrainer` API is only accessible with a
separate import `ray.train.xgboost.v2.XGBoostTrainer`.

To avoid unnecessary import changes, this PR folds the new API, which
accepts new arguments `(train_loop_per_worker, train_loop_config,
xgboost_config)`, into the public `ray.train.xgboost.XGBoostTrainer`
class.

This also makes some changes in the Ray Train v2 `XGBoostTrainer` class
to improve the migration UX, since it does not support the legacy
`XGBoostTrainer` API at all.

---------

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
park12sj pushed a commit to park12sj/ray that referenced this pull request Mar 18, 2025
… an alternate constructor (ray-project#50045)

Currently, the new `XGBoostTrainer` API is only accessible with a
separate import `ray.train.xgboost.v2.XGBoostTrainer`.

To avoid unnecessary import changes, this PR folds the new API, which
accepts new arguments `(train_loop_per_worker, train_loop_config,
xgboost_config)`, into the public `ray.train.xgboost.XGBoostTrainer`
class.

This also makes some changes in the Ray Train v2 `XGBoostTrainer` class
to improve the migration UX, since it does not support the legacy
`XGBoostTrainer` API at all.

---------

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
dhakshin32 pushed a commit to dhakshin32/ray that referenced this pull request Mar 27, 2025
… an alternate constructor (ray-project#50045)

Currently, the new `XGBoostTrainer` API is only accessible with a
separate import `ray.train.xgboost.v2.XGBoostTrainer`.

To avoid unnecessary import changes, this PR folds the new API, which
accepts new arguments `(train_loop_per_worker, train_loop_config,
xgboost_config)`, into the public `ray.train.xgboost.XGBoostTrainer`
class.

This also makes some changes in the Ray Train v2 `XGBoostTrainer` class
to improve the migration UX, since it does not support the legacy
`XGBoostTrainer` API at all.

---------

Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Dhakshin Suriakannu <d_suriakannu@apple.com>
justinvyu added a commit that referenced this pull request Apr 9, 2025
…s as an alternate constructor (#51265)

This PR is a follow-up of #50045,
which folds the new LightGBM API into
`ray.train.lightgbm.LightGBMTrainer` so that users don't need to change
their imports to use the new custom train function API.

---------

Signed-off-by: Hongpeng Guo <hpguo@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
han-steve pushed a commit to han-steve/ray that referenced this pull request Apr 11, 2025
…s as an alternate constructor (ray-project#51265)

This PR is a follow-up of ray-project#50045,
which folds the new LightGBM API into
`ray.train.lightgbm.LightGBMTrainer` so that users don't need to change
their imports to use the new custom train function API.

---------

Signed-off-by: Hongpeng Guo <hpguo@anyscale.com>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
Co-authored-by: Justin Yu <justinvyu@anyscale.com>
Signed-off-by: Steve Han <stevehan2001@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-backlog go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants