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.

Sign up for GitHub

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

Jump to bottom

[python-package] make _InnerPredictor construction stricter #5961

Merged
merged 6 commits into from
Jul 14, 2023
Merged

[python-package] make _InnerPredictor construction stricter #5961

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4e5d8bb
[python-package] make _InnerPredictor construction stricter
jameslamb Jul 7, 2023
fadb605
make handle non-None
jameslamb Jul 8, 2023
0f92863
docstring
jameslamb Jul 8, 2023
fb06b65
Apply suggestions from code review
jameslamb Jul 12, 2023
0234971
fix docstrings
jameslamb Jul 12, 2023
2d9ded2
Merge branch 'master' into simplify-inner-predictor
jameslamb Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 95 additions & 52 deletions python-package/lightgbm/basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
'Sequence',
]

_BoosterHandle = ctypes.c_void_p
_DatasetHandle = ctypes.c_void_p
_ctypes_int_ptr = Union[
"ctypes._Pointer[ctypes.c_int32]",
Expand Down Expand Up @@ -837,52 +838,98 @@ class _InnerPredictor:

def __init__(
self,
model_file: Optional[Union[str, Path]] = None,
booster_handle: Optional[ctypes.c_void_p] = None,
pred_parameter: Optional[Dict[str, Any]] = None
booster_handle: _BoosterHandle,
pandas_categorical: Optional[List[List]],
pred_parameter: Dict[str, Any],
manage_handle: bool
):
"""Initialize the _InnerPredictor.

Parameters
----------
model_file : str, pathlib.Path or None, optional (default=None)
Path to the model file.
booster_handle : object or None, optional (default=None)
booster_handle : Booster
jmoralez marked this conversation as resolved.
Show resolved Hide resolved
Handle of Booster.
pred_parameter: dict or None, optional (default=None)
pandas_categorical : list of list, or None
If provided, list of categories for ``pandas`` categorical columns.
Where the ``i``th element of the list contains the categories for the ``i``th categorical feature.
pred_parameter: dict
Other parameters for the prediction.
manage_handle : bool
If ``True``, free the corresponding Booster on the C++ side when this Python object is deleted.
"""
self._handle = ctypes.c_void_p()
self.__is_manage_handle = True
if model_file is not None:
"""Prediction task"""
out_num_iterations = ctypes.c_int(0)
_safe_call(_LIB.LGBM_BoosterCreateFromModelfile(
_c_str(str(model_file)),
ctypes.byref(out_num_iterations),
ctypes.byref(self._handle)))
out_num_class = ctypes.c_int(0)
_safe_call(_LIB.LGBM_BoosterGetNumClasses(
self._handle,
ctypes.byref(out_num_class)))
self.num_class = out_num_class.value
self.num_total_iteration = out_num_iterations.value
self.pandas_categorical = _load_pandas_categorical(file_name=model_file)
elif booster_handle is not None:
self.__is_manage_handle = False
self._handle = booster_handle
out_num_class = ctypes.c_int(0)
_safe_call(_LIB.LGBM_BoosterGetNumClasses(
self._handle = booster_handle
self.__is_manage_handle = manage_handle
self.pandas_categorical = pandas_categorical
self.pred_parameter = _param_dict_to_str(pred_parameter)

out_num_class = ctypes.c_int(0)
_safe_call(
_LIB.LGBM_BoosterGetNumClasses(
self._handle,
ctypes.byref(out_num_class)))
self.num_class = out_num_class.value
self.num_total_iteration = self.current_iteration()
self.pandas_categorical = None
else:
raise TypeError('Need model_file or booster_handle to create a predictor')
ctypes.byref(out_num_class)
)
)
self.num_class = out_num_class.value

pred_parameter = {} if pred_parameter is None else pred_parameter
self.pred_parameter = _param_dict_to_str(pred_parameter)
@classmethod
def from_booster(
cls,
booster: "Booster",
pred_parameter: Dict[str, Any]
) -> "_InnerPredictor":
"""Initialize an ``_InnerPredictor`` from a ``Booster``.

Parameters
----------
booster : Booster
Booster.
pred_parameter: dict
Other parameters for the prediction.
"""
out_cur_iter = ctypes.c_int(0)
_safe_call(
_LIB.LGBM_BoosterGetCurrentIteration(
booster._handle,
ctypes.byref(out_cur_iter)
)
)
return cls(
booster_handle=booster._handle,
pandas_categorical=booster.pandas_categorical,
pred_parameter=pred_parameter,
manage_handle=False
)

@classmethod
def from_model_file(
cls,
model_file: Union[str, Path],
pred_parameter: Dict[str, Any]
) -> "_InnerPredictor":
"""Initialize an ``_InnerPredictor`` from a text file containing a LightGBM model.

Parameters
----------
model_file : str or pathlib.Path
Path to the model file.
pred_parameter: dict
Other parameters for the prediction.
"""
booster_handle = ctypes.c_void_p()
out_num_iterations = ctypes.c_int(0)
_safe_call(
_LIB.LGBM_BoosterCreateFromModelfile(
_c_str(str(model_file)),
ctypes.byref(out_num_iterations),
ctypes.byref(booster_handle)
)
)
return cls(
booster_handle=booster_handle,
pandas_categorical=_load_pandas_categorical(file_name=model_file),
pred_parameter=pred_parameter,
manage_handle=True
)

def __del__(self) -> None:
try:
Expand Down Expand Up @@ -3047,7 +3094,7 @@ def __init__(
model_str : str or None, optional (default=None)
Model will be loaded from this string.
"""
self._handle = None
self._handle = ctypes.c_void_p()
self._network = False
self.__need_reload_eval_info = True
self._train_data_name = "training"
Expand Down Expand Up @@ -3098,7 +3145,6 @@ def __init__(
# copy the parameters from train_set
params.update(train_set.get_params())
params_str = _param_dict_to_str(params)
self._handle = ctypes.c_void_p()
_safe_call(_LIB.LGBM_BoosterCreate(
train_set._handle,
_c_str(params_str),
Expand Down Expand Up @@ -3127,7 +3173,6 @@ def __init__(
elif model_file is not None:
# Prediction task
out_num_iterations = ctypes.c_int(0)
self._handle = ctypes.c_void_p()
_safe_call(_LIB.LGBM_BoosterCreateFromModelfile(
_c_str(str(model_file)),
ctypes.byref(out_num_iterations),
Expand Down Expand Up @@ -3906,8 +3951,9 @@ def model_from_string(self, model_str: str) -> "Booster":
self : Booster
Loaded Booster object.
"""
if self._handle is not None:
_safe_call(_LIB.LGBM_BoosterFree(self._handle))
# ensure that existing Booster is freed before replacing it
# with a new one createdfrom file
_safe_call(_LIB.LGBM_BoosterFree(self._handle))
Comment on lines -3908 to +3955
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As of #5947, there is no path through lightgbm's public API which would allow a user to overwrite Booster._handle with None.

Making these changes to ensure it's never None avoids the need to do if {booster}._handle is not None types of checks in _InnerPredictor.from_booster().

self._free_buffer()
self._handle = ctypes.c_void_p()
out_num_iterations = ctypes.c_int(0)
Expand Down Expand Up @@ -4107,7 +4153,10 @@ def predict(
Prediction result.
Can be sparse or a list of sparse objects (each element represents predictions for one class) for feature contributions (when ``pred_contrib=True``).
"""
predictor = self._to_predictor(pred_parameter=deepcopy(kwargs))
predictor = _InnerPredictor.from_booster(
booster=self,
pred_parameter=deepcopy(kwargs),
)
if num_iteration is None:
if start_iteration <= 0:
num_iteration = self.best_iteration
Expand Down Expand Up @@ -4224,7 +4273,10 @@ def refit(
raise LightGBMError('Cannot refit due to null objective function.')
if dataset_params is None:
dataset_params = {}
predictor = self._to_predictor(pred_parameter=deepcopy(kwargs))
predictor = _InnerPredictor.from_booster(
booster=self,
pred_parameter=deepcopy(kwargs)
)
leaf_preds: np.ndarray = predictor.predict( # type: ignore[assignment]
data=data,
start_iteration=-1,
Expand Down Expand Up @@ -4328,15 +4380,6 @@ def set_leaf_output(
)
return self

def _to_predictor(
self,
pred_parameter: Dict[str, Any]
) -> _InnerPredictor:
"""Convert to predictor."""
predictor = _InnerPredictor(booster_handle=self._handle, pred_parameter=pred_parameter)
predictor.pandas_categorical = self.pandas_categorical
return predictor

def num_feature(self) -> int:
"""Get number of features.

Expand Down
26 changes: 21 additions & 5 deletions python-package/lightgbm/engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,20 @@ def train(

predictor: Optional[_InnerPredictor] = None
if isinstance(init_model, (str, Path)):
predictor = _InnerPredictor(model_file=init_model, pred_parameter=params)
predictor = _InnerPredictor.from_model_file(
model_file=init_model,
pred_parameter=params
)
elif isinstance(init_model, Booster):
predictor = init_model._to_predictor(pred_parameter=dict(init_model.params, **params))
init_iteration = predictor.num_total_iteration if predictor is not None else 0
predictor = _InnerPredictor.from_booster(
booster=init_model,
pred_parameter=dict(init_model.params, **params)
)

if predictor is not None:
init_iteration = predictor.current_iteration()
else:
init_iteration = 0

train_set._update_params(params) \
._set_predictor(predictor) \
Expand Down Expand Up @@ -685,9 +695,15 @@ def cv(
first_metric_only = params.get('first_metric_only', False)

if isinstance(init_model, (str, Path)):
predictor = _InnerPredictor(model_file=init_model, pred_parameter=params)
predictor = _InnerPredictor.from_model_file(
model_file=init_model,
pred_parameter=params
)
elif isinstance(init_model, Booster):
predictor = init_model._to_predictor(pred_parameter=dict(init_model.params, **params))
predictor = _InnerPredictor.from_booster(
booster=init_model,
pred_parameter=dict(init_model.params, **params)
)
else:
predictor = None

Expand Down