Skip to content
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

[train] Simplify ray.train.xgboost/lightgbm (5/n): Remove xgboost_ray and lightgbm_ray dependencies (for release tests) #43425

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

justinvyu
Copy link
Contributor

@justinvyu justinvyu commented Feb 26, 2024

Why are these changes needed?

This PR removes release tests for the external xgboost_ray and lightgbm_ray packages that are no longer used by the new implementation of XGBoostTrainer and LightGBMTrainer. See #42767 and #43244 for more details on the new implementations.

air_benchmark_xgboost_cpu_10 is the only release test that actually tests XGBoostTrainer, which has been kept around. TODO: A follow-up PR will add 2 more release tests (for each framework) to add back test coverage: a training and a tuning release test that uses XGBoostTrainer/LightGBMTrainer in a multi-node cluster.

Related issue number

Checks

  • I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
  • I've run scripts/format.sh to lint the changes in this PR.
  • I've included any doc changes needed for https://docs.ray.io/en/master/.
    • I've added any new APIs to the API Reference. For example, if I added a
      method in Tune, I've added it in doc/source/tune/api/ under the
      corresponding .rst file.
  • I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
  • Testing Strategy
    • Unit tests
    • Release tests
    • This PR is not tested :(

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>
Signed-off-by: Justin Yu <justinvyu@anyscale.com>
@justinvyu justinvyu requested a review from a team as a code owner February 26, 2024 09:13
@can-anyscale
Copy link
Collaborator

There are some lint errors but otherwise exciting! Let's run some release tests to validate. Thankks.

Copy link
Collaborator

@aslonnie aslonnie left a comment

Choose a reason for hiding this comment

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

for @can-anyscale to review

@justinvyu justinvyu changed the title [train] Simplify ray.train.xgboost/lightgbm (5/n): Remove xgboost_ray and lightgbm_ray dependencies (for releaase tests) [train] Simplify ray.train.xgboost/lightgbm (5/n): Remove xgboost_ray and lightgbm_ray dependencies (for release tests) Feb 27, 2024
Copy link
Collaborator

@can-anyscale can-anyscale left a comment

Choose a reason for hiding this comment

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

failing lint; do you want to run some release tests to validate the changes as well, thankks

Copy link
Collaborator

@can-anyscale can-anyscale left a comment

Choose a reason for hiding this comment

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

@justinvyu
Copy link
Contributor Author

cc @woshiyyya @matthewdeng I am removing these for now because they're just testing xgboost_ray/lightgbm_ray but will add back 2 release tests in the next PR.

@justinvyu
Copy link
Contributor Author

The remaining XGBoostTrainer release test passed here without installing xgboost_ray!

https://buildkite.com/ray-project/release/builds/9540#018dec19-aae3-4d0b-9368-0504d37b7d72

@justinvyu justinvyu merged commit aed14c8 into ray-project:master Feb 27, 2024
9 checks passed
@justinvyu justinvyu deleted the remove_xgb_lgbm_ray_release branch February 27, 2024 23:13
@justinvyu justinvyu mentioned this pull request Feb 28, 2024
8 tasks
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.

None yet

5 participants