-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
[python-package] ensure that all callbacks are pickleable #5080
Comments
Closing this issue and adding it to #2302, per this project's policy for feature requests. Anyone is welcome to contribute this, and a fix can be done across multiple pull requests. If you're interested in working on this, please comment below and maintainers will re-open the issue. |
@jameslamb Thanks a lot for writing this up!
Let me kindly disagree with this statement. I believe we should refactor current interface converting it into classes anyway for the sake of code readability and to avoid any unintended side effects. |
Also, I think it worth mentioning that there shouldn't be any code refactoring that assumes that all callbacks are pickleable. We should allow to use third-party callbacks that are unpickleable: #4719. |
I don't disagree with the idea that all callbacks should be callable classes. I just think that's larger than the scope of what I'm proposing in this issue. If we want to change the callbacks to callable classes for reasons other than pickleability, then I'd consider that separate from this work. If you'd prefer to change this feature request to "convert all callbacks to callable classes", and add that one of the conditions for that is "test that all of them are pickleable", that's fine with me. I just was thinking it made it a bit easier to separate those two concerns.
Good clarification. I was referring only to those built into the |
This issue has been automatically locked since there has not been any recent activity since it was closed. |
Summary
All callback functions should be serializable with
pickle
,clooudpickle
, andjoblib
.Motivation
As described in #5012, in some interfaces to LightGBM, it's necessary to broadcast LightGBM parameters or objects from the
lightgbm
Python library to multiple processes / machines. The primary mechanism for this in Python is to serialize objects with a library likepickle
,cloudpickle
, orjoblib
.#5012 made
lgb.callback.early_stopping
serializable, for the benefit of lightgbm-ray. I expect that would also be necessary to use callbacks inlightgbm.dask
, and for any other settings where users want to pickle/unpickle LightGBM objects.Description
Functions in
lightgbm.callback
:early_stopping()
([python] makeearly_stopping
callback pickleable #5012)log_evaluation()
([python] makelog_evaluation
callback pickleable #5101)record_evaluation()
([python] makerecord_evaluation
callback pickleable #5107)reset_parameter()
([python] makereset_parameter
callback pickleable #5109)Tests similar to the following should be added for each of these functions, to ensure that they're pickleable and that that remains true after future changes to this project.
LightGBM/tests/python_package_test/test_callback.py
Lines 9 to 22 in f77e0ad
It's possible that just adding such tests will be enough, and that the remaining functions are already pickleable. But if not, then changes will need to be made to support this.
The text was updated successfully, but these errors were encountered: