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

jwyyy · 2021-11-30T18:48:38Z

What changes are proposed in this pull request?

This PR enables the support for saving and loading all LightGBM models.

(Draft + discussion: #4296, #4885. Template: #4954)

How is this patch tested?

New test methods are added.

Does this PR change the documentation?

No. You can skip the rest of this section.
Yes. Make sure the changed pages / sections render correctly by following the steps below.

Check the status of the ci/circleci: build_doc check. If it's successful, proceed to the
next step, otherwise fix it.
Click Details on the right to open the job page of CircleCI.
Click the Artifacts tab.
Click docs/build/html/index.html.
Find the changed pages / sections and make sure they render correctly.

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 and loading LightGBM models, including sklearn models, with model class specification.
Functions save_model() / load_model() in mlflow.lightgbm 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-11-30T20:32:38Z

Hi @dbczumar @harupy, I made the first PR to support autologging for LightGBM sklearn model. Unlike XGBoost sklearn models, LightGBM sklearn models don't support save_model() and load_model() currently. It needs to be done by obtaining internal Boosters first. So I made a tentative plan by (1) re-organizing the lightgbm folder (2) and adding utils methods to handle LightGBM sklearn model saving / loading. We can revise the logic once microsoft/LightGBM#4841 is addressed. To enable loading sklearn models, we need to load Booster / sklearn model parameters. Currently this is not fully supported either. (An ongoing PR microsoft/LightGBM#4802 is trying to fix this for Boosters.) As a workaround, this PR saves Booster / sklearn model parameters in JSON. When loading back models, these parameters will be read in to recover the attributes of saved models.

Please let me your feedback and suggestions when you have time to review it! Thank you so much!

jwyyy · 2021-12-13T19:10:55Z

Hi @dbczumar @harupy, it seems the LightGBM community doesn't have a plan to implement save_model() and load_model for sklearn models (it's been a while since I opened the issue but no conclusion was reached). They recommended to use pickle or joblib, which is inconsistent with Booster saving / loading but same as the current implementation of model saving in mlflow.sklearn. Please let me know what your thoughts and comments are. I will revise the current PR. Thanks a lot!

dbczumar

@jwyyy Excellent work! Apologies for the delayed response due to a busy holiday season. Regarding your question about serialization / deserialization, I think it's probably best to use pickle for now. This is less likely to break across LightGBM versions. Can we test out serialization / deserialization with custom objective functions (see docs for eval_metric here: https://lightgbm.readthedocs.io/en/latest/pythonapi/lightgbm.LGBMClassifier.html#lightgbm.LGBMClassifier.fit) and make sure that pickle works successfully? If it doesn't, we may want to use cloudpickle.

mlflow/lightgbm/__init__.py

dbczumar · 2021-12-14T07:51:37Z

mlflow/lightgbm/utils.py

+        return None
+
+
+def _save_lgb_model(lgb_model, model_path) -> None:


Following from #5130 (comment), I think it's probably safest to use pickle to save / load the model, since the LightGBM developers could make breaking changes that break the serialization / deserialization code. Thank you so much for taking the time to reach out to the LightGBM community and get some insight into the recommended best practices here.

Can we test out serialization / deserialization with custom objective functions (see docs for eval_metric here: https://lightgbm.readthedocs.io/en/latest/pythonapi/lightgbm.LGBMClassifier.html#lightgbm.LGBMClassifier.fit) and make sure that pickle works successfully? If it doesn't, we may want to use cloudpickle.

jwyyy · 2021-12-14T19:04:56Z

Hi @dbczumar, no worries! Thank you for your review and feedback! I will update the PR accordingly.

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

jwyyy · 2021-12-16T00:48:06Z

Hi @dbczumar @harupy, I made some changes to the PR. For serialization / deserialization of LightGBM sklearn models, I used cloudpickle instead of pickle. Custom objective functions for the argument eval_metric work fine with pickle, because eval_metric is only called during training and not assigned to any class member. However, pickle doesn't work with the custom objective (Python lambda functions) in LightGBM sklearn model initialization. Would love to learn your feedback and comments. Thanks!

harupy · 2021-12-21T01:06:52Z

mlflow/lightgbm.py

@@ -73,7 +74,7 @@ def get_default_pip_requirements():
             Calls to :func:`save_model()` and :func:`log_model()` produce a pip environment
             that, at minimum, contains these requirements.
    """
-    return [_get_pinned_requirement("lightgbm")]
+    return [_get_pinned_requirement("lightgbm"), _get_pinned_requirement("cloudpickle")]


Can we add cloudpickle conditionally because users who don't use scikit-learn estimators don't need cloudpickle?

Hi @harupy, thank you for your suggestion! Does that mean we also need to provide an option to turn on / off autologging for scikit-learn estimators? I assumed mlflow.lightgbm.autolog() enables autologging for all models.

Hi @harupy, I found a simple way to add cloudpickle conditionally (and automatically) based on what model is saved (please see L169-171). Please let me know your feedback and comments. Thanks a lot!

(This comment is also addressed in the latest commit.)

harupy · 2021-12-21T01:24:02Z

mlflow/lightgbm.py

+    if isinstance(lgb_model, lgb.Booster):
+        model_data_subpath = "model.lgb"
+    else:
+        model_data_subpath = "model.pkl"


Suggested change

if isinstance(lgb_model, lgb.Booster):

model_data_subpath = "model.lgb"

else:

model_data_subpath = "model.pkl"

model_data_subpath = "model.lgb" if isinstance(lgb_model, lgb.Booster) else "model.pkl"

nit

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

harupy

looks good to me!

jwyyy · 2021-12-23T15:32:12Z

@harupy Thank you for your review!

jwyyy added 2 commits November 30, 2021 10:38

init commit

ce9c5e3

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

restore test

af4bcf1

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

github-actions bot added area/models MLmodel format, model serialization/deserialization, flavors rn/feature Mention under Features in Changelogs. labels Nov 30, 2021

fix doc

c804a81

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

dbczumar reviewed Dec 14, 2021

View reviewed changes

jwyyy added 2 commits December 15, 2021 16:04

address review: use cloudpickle

aa4337b

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

remove prev folders

7397fce

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

harupy reviewed Dec 21, 2021

View reviewed changes

address review

c212f55

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

jwyyy requested a review from harupy December 22, 2021 17:41

a better soln

2b31029

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

harupy approved these changes Dec 23, 2021

View reviewed changes

harupy merged commit 4c657b3 into mlflow:master Dec 24, 2021

jwyyy deleted the lightgbm_save_load branch December 24, 2021 06:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jwyyy commented Nov 30, 2021

jwyyy commented Nov 30, 2021

jwyyy commented Dec 13, 2021

dbczumar left a comment

dbczumar Dec 14, 2021 •

edited

jwyyy commented Dec 14, 2021

jwyyy commented Dec 16, 2021

harupy Dec 21, 2021

jwyyy Dec 21, 2021 •

edited

jwyyy Dec 22, 2021 •

edited

harupy Dec 21, 2021

harupy left a comment

jwyyy commented Dec 23, 2021

		return None


		def _save_lgb_model(lgb_model, model_path) -> None:

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

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

Conversation

jwyyy commented Nov 30, 2021

What changes are proposed in this pull request?

How is this patch tested?

Does this PR change the documentation?

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:

jwyyy commented Nov 30, 2021

jwyyy commented Dec 13, 2021

dbczumar left a comment

Choose a reason for hiding this comment

dbczumar Dec 14, 2021 • edited

Choose a reason for hiding this comment

jwyyy commented Dec 14, 2021

jwyyy commented Dec 16, 2021

harupy Dec 21, 2021

Choose a reason for hiding this comment

jwyyy Dec 21, 2021 • edited

Choose a reason for hiding this comment

jwyyy Dec 22, 2021 • edited

Choose a reason for hiding this comment

harupy Dec 21, 2021

Choose a reason for hiding this comment

harupy left a comment

Choose a reason for hiding this comment

jwyyy commented Dec 23, 2021

dbczumar Dec 14, 2021 •

edited

jwyyy Dec 21, 2021 •

edited

jwyyy Dec 22, 2021 •

edited