Autologging functionality for scikit-learn integration with XGBoost (Part 1) by jwyyy · Pull Request #4954 · mlflow/mlflow

jwyyy · 2021-10-28T04:14:31Z

Signed-off-by: Junwen Yao jwyiao@gmail.com

What changes are proposed in this pull request?

This is the first PR to add autologging for XGBoost sklearn models. It revises model saving / loading and adds model_class to specify XGBoost model class, such as Booster, or XGBRegressor.

A separate PR will be made to add autologging for XGBoost sklearn models using mlflow.sklearn routine.

(Draft + discussion: #4885)

How is this patch tested?

New test functions are added. See PR files.

Release Notes

Is this a user-facing change?

No. You can skip the rest of this section.
Yes. Give a description of this change to be included in the release notes for MLflow users.

This PR will enable saving / loading XGBoost models, including sklearn models, with model class specification.
Functions save_model() / load_model() in mlflow.xgboost can be used as before.

What component(s), interfaces, languages, and integrations does this PR affect?

Components

Interface

area/uiux: Front-end, user experience, plotting, JavaScript, JavaScript dev server
area/docker: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
area/sqlalchemy: Use of SQLAlchemy in the Tracking Service or Model Registry
area/windows: Windows support

Language

language/r: R APIs and clients
language/java: Java APIs and clients
language/new: Proposals for new client languages

Integrations

integrations/azure: Azure and Azure ML integrations
integrations/sagemaker: SageMaker integrations
integrations/databricks: Databricks integrations

How should the PR be classified in the release notes? Choose one:

rn/breaking-change - The PR will be mentioned in the "Breaking Changes" section
rn/none - No description will be included. The PR will be mentioned only by the PR number in the "Small Bugfixes and Documentation Updates" section
rn/feature - A new user-facing feature worth mentioning in the release notes
rn/bug-fix - A user-facing bug fix worth mentioning in the release notes
rn/documentation - A user-facing documentation change worth mentioning in the release notes

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy · 2021-10-28T04:21:41Z

Hi @dbczumar, as per our discussion, I separate the autologging functionality and model saving / loading with model class specification into two PRs. This is the first one to address model saving / loading. Please let me know if I missed anything. Thanks!

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

dbczumar

@jwyyy Looks awesome! I left a few tiny comments - I think this is very close!

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy · 2021-10-30T16:48:34Z

Hi @dbczumar @harupy, thank you for your feedback! I updated this PR and doc according to your suggestions. Please let me know if they look good. Thanks!

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy · 2021-11-02T16:25:20Z

Hi @harupy @dbczumar, thank you for initializing the auto check! According to the error messages, two unsuccessful checks were due to network connection exception, not related to the changes in this PR. I am not sure if I can fix these two problems efficiently. Can you take a look and re-try the check? Thanks! Also please let me know if the latest commit look good.

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy · 2021-11-07T06:21:00Z

Hi @harupy @dbczumar, I notice that this PR #4997 tries to add a new argument to load_model() in every flavor. This will have an impact on how we handle model loading in mlflow.xgboost (although the revision should be very easy). I am wondering which PR will be addressed first. Or maybe we can directly add the xgboost part in this PR. Please let you know your plan / ideas. Thanks!

(The previous failed check (35/36) was due to Keras 2.7 compatibility, which should be fixed now.)

harupy · 2021-11-07T09:20:35Z

I notice that this PR #4997 tries to add a new argument to load_model() in every flavor.

Yes it does, but the new argument doesn't change how to load the model, it just allows users to determine where to load the model. Therefore, it should not conflict with this PR.

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

harupy

Looks good to me once #4954 (comment) is addressed!

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy · 2021-11-09T06:39:41Z

Looks good to me once #4954 (comment) is addressed!

@harupy I just updated the comments in this PR, including #4954 (review).

jwyyy · 2021-11-09T06:47:59Z

@harupy @dbczumar Thank you very much for your feedback and suggestions! It seems we are at the final stage of this PR. Once it is merged, I will make the second PR which completes the autologging for XGBoost sklearn models based on our previous discussion. It should be ready within the next few days. Meanwhile I will also submit the first PR for LightGBM models (similar to this PR). Thanks again!

dbczumar

LGTM! Thanks so much @jwyyy !

jwyyy · 2021-11-10T02:02:21Z

Two failed tests seem unrelated to this PR. The error messages are the same as unsuccessful checks in recent commits to master #5032 #5031 #4999 #5029.

harupy · 2021-11-10T02:16:51Z

Two failed tests seem unrelated to this PR.

Yep, we're already investigating them. Let's merge this PR!

xgb_sklearn_save_load

b6333b7

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

github-actions Bot added area/artifacts Artifact stores and artifact logging area/models MLmodel format, model serialization/deserialization, flavors labels Oct 28, 2021

harupy reviewed Oct 28, 2021

View reviewed changes

Comment thread mlflow/xgboost.py

harupy reviewed Oct 28, 2021

View reviewed changes

Comment thread tests/xgboost/test_xgboost_model_export.py Outdated

harupy reviewed Oct 28, 2021

View reviewed changes

Comment thread tests/xgboost/test_xgboost_model_export.py Outdated

github-actions Bot added rn/documentation Mention under Documentation Changes in Changelogs. rn/feature Mention under Features in Changelogs. and removed rn/documentation Mention under Documentation Changes in Changelogs. labels Oct 28, 2021

address review + fix lint

c542344

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy requested a review from harupy October 28, 2021 15:21

dbczumar reviewed Oct 29, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

dbczumar reviewed Oct 29, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

dbczumar reviewed Oct 29, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

dbczumar reviewed Oct 29, 2021

View reviewed changes

address review + update doc

aa6d741

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy commented Oct 30, 2021

View reviewed changes

Comment thread mlflow/xgboost.py

harupy reviewed Nov 1, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

harupy reviewed Nov 1, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

harupy reviewed Nov 1, 2021

View reviewed changes

Comment thread tests/xgboost/test_xgboost_model_export.py Outdated

address review

19940e6

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy commented Nov 1, 2021

View reviewed changes

Comment thread tests/xgboost/test_xgboost_model_export.py Outdated

harupy reviewed Nov 3, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

jwyyy added 2 commits November 3, 2021 09:06

address review, keep cmt

7bea161

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

address review

a7156cd

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

jwyyy requested review from dbczumar and harupy November 4, 2021 23:45

harupy reviewed Nov 7, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

jwyyy added 6 commits November 8, 2021 14:19

Merge branch 'mlflow:master' into xgb_save_load

63a6bf5

address review: add back data in pyfunc

275f2e9

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

update doc

396a451

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

update cmt

eb6a966

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

resolve conflict in master

0fa7b83

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

Merge branch 'master' into xgb_save_load

4471984

harupy reviewed Nov 9, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

harupy approved these changes Nov 9, 2021

View reviewed changes

harupy reviewed Nov 9, 2021

View reviewed changes

Comment thread mlflow/xgboost.py Outdated

address review: update cmt

4e55a30

Signed-off-by: Junwen Yao <jwyiao@gmail.com>

dbczumar approved these changes Nov 10, 2021

View reviewed changes

harupy merged commit 1fa5433 into mlflow:master Nov 10, 2021

jwyyy deleted the xgb_save_load branch November 10, 2021 03:19

This was referenced Nov 12, 2021

Autologging functionality for scikit-learn integration with XGBoost (Part 2) #5055

Closed

Autologging functionality for scikit-learn integration with XGBoost (Part 2) #5078

Merged

jwyyy mentioned this pull request Nov 30, 2021

Autologging functionality for scikit-learn integration with LightGBM (Part 1) #5130

Merged

28 tasks

harupy mentioned this pull request Jun 3, 2022

XGBoost support #4204

Open

5 tasks

Conversation

jwyyy commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes are proposed in this pull request?

How is this patch tested?

Release Notes

Is this a user-facing change?

What component(s), interfaces, languages, and integrations does this PR affect?

How should the PR be classified in the release notes? Choose one:

Uh oh!

jwyyy commented Oct 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dbczumar left a comment

Choose a reason for hiding this comment

Uh oh!

jwyyy commented Oct 30, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jwyyy commented Nov 2, 2021

Uh oh!

Uh oh!

jwyyy commented Nov 7, 2021

Uh oh!

harupy commented Nov 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

harupy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jwyyy commented Nov 9, 2021

Uh oh!

jwyyy commented Nov 9, 2021

Uh oh!

dbczumar left a comment

Choose a reason for hiding this comment

Uh oh!

jwyyy commented Nov 10, 2021

Uh oh!

harupy commented Nov 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jwyyy commented Oct 28, 2021 •

edited

Loading

harupy commented Nov 7, 2021 •

edited

Loading

harupy left a comment •

edited

Loading