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

support NumPy 2.0 (fixes #560) #562

Merged
merged 2 commits into from
May 23, 2024
Merged

support NumPy 2.0 (fixes #560) #562

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c67cf9b
support NumPy 2.0 (fixes #560)
jameslamb May 16, 2024
161253c
Merge branch 'mainline' into numpy-2.0
hcho3 May 23, 2024
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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,5 @@ __pycache__
/python/.idea/
/tests/python/.idea/
/.idea/
/.hypothesis
/lint.py
5 changes: 5 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,8 @@ repos:
hooks:
- id: mypy
additional_dependencies: [types-setuptools, numpy]
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.4
hooks:
- id: ruff
args: ["--config", "python/pyproject.toml"]
12 changes: 12 additions & 0 deletions python/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,15 @@ testing = ["scikit-learn", "pytest", "hypothesis", "pandas"]
plugins = "numpy.typing.mypy_plugin"

[tool.hatch.build.targets.wheel.hooks.custom]

[tool.ruff]
line-length = 120

# this should be set to the oldest version of python treelite supports
target-version = "py38"

[tool.ruff.lint]
select = [
# numpy 2.0 deprecations/removals
"NPY201",
]
11 changes: 5 additions & 6 deletions python/treelite/gtil/gtil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ def _predict_impl(
is_dense = isinstance(data, np.ndarray)

if is_dense:
data = np.array(
data, copy=False, dtype=typestr_to_numpy_type(model.input_type), order="C"
data = np.asarray(
data, dtype=typestr_to_numpy_type(model.input_type), order="C"
)
if data.shape[1] < model.num_feature:
# Pad missing features with NAs
Expand All @@ -177,14 +177,13 @@ def _predict_impl(
assert data.shape[1] == model.num_feature
else:
assert isinstance(data, csr_matrix)
elems = np.array(
elems = np.asarray(
data.data,
copy=False,
dtype=typestr_to_numpy_type(model.input_type),
order="C",
)
col_ind = np.array(data.indices, copy=False, dtype=np.uint64, order="C")
row_ptr = np.array(data.indptr, copy=False, dtype=np.uint64, order="C")
col_ind = np.asarray(data.indices, dtype=np.uint64, order="C")
row_ptr = np.asarray(data.indptr, dtype=np.uint64, order="C")
output_shape_ptr = ctypes.POINTER(ctypes.c_uint64)()
output_ndim = ctypes.c_uint64()
_check_call(
Expand Down
2 changes: 1 addition & 1 deletion python/treelite/sklearn/importer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def add(self, array, *, expected_shape=None):
assert (
array.shape == expected_shape
), f"Expected shape: {expected_shape}, Got shape {array.shape}"
v = np.array(array, copy=False, dtype=self.dtype, order="C")
v = np.asarray(array, dtype=self.dtype, order="C")
self.collection.append(v)

def as_c_array(self):
Expand Down
2 changes: 1 addition & 1 deletion src/c_api/model.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ int TreeliteConcatenateModelObjects(
API_BEGIN();
std::vector<treelite::Model const*> model_objs(len, nullptr);
std::transform(objs, objs + len, model_objs.begin(),
[](TreeliteModelHandle e) { return static_cast<const treelite::Model*>(e); });
[](TreeliteModelHandle e) { return static_cast<treelite::Model const*>(e); });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

clang-format made these style changes to C++ files automatically when I ran

pre-commit run --all-files

auto concatenated_model = ConcatenateModelObjects(model_objs);
*out = static_cast<TreeliteModelHandle>(concatenated_model.release());
API_END();
Expand Down
2 changes: 1 addition & 1 deletion src/model_concat.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ std::unique_ptr<Model> ConcatenateModelObjects(std::vector<Model const*> const&
auto& casted = std::get<ModelType>(objs[i]->variant_);
std::transform(casted.trees.begin(), casted.trees.end(),
std::back_inserter(concatenated_model_concrete.trees),
[](const auto& tree) { return tree.Clone(); });
[](auto const& tree) { return tree.Clone(); });
concatenated_model->target_id.Extend(objs[i]->target_id);
concatenated_model->class_id.Extend(objs[i]->class_id);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/python/test_lightgbm_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def test_lightgbm_sparse_ranking_model(tmpdir):

lgb_model_path = pathlib.Path(tmpdir) / "sparse_ranking_lightgbm.txt"

dtrain = lgb.Dataset(X, label=y, group=[X.shape[0]])
dtrain = lgb.Dataset(X, label=y, group=np.array([X.shape[0]], dtype=np.int32))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one is lightgbm's responsibility to fix.

Passing a list for group ends up invoking np.array(..., copy=False).

    def _list_to_1d_numpy(
        data: Any,
        dtype: "np.typing.DTypeLike",
        name: str
    ) -> np.ndarray:
        """Convert data to numpy 1-D array."""
        if _is_numpy_1d_array(data):
            return _cast_numpy_array_to_dtype(data, dtype)
        elif _is_numpy_column_array(data):
            _log_warning('Converting column-vector to 1d array')
            array = data.ravel()
            return _cast_numpy_array_to_dtype(array, dtype)
        elif _is_1d_list(data):
>           return np.array(data, dtype=dtype, copy=False)
E           ValueError: Unable to avoid copy while creating an array as requested.
E           If using `np.array(obj, copy=False)` replace it with `np.asarray(obj)` to allow a copy when needed (no behavior change in NumPy 1.x).
E           For more details, see https://numpy.org/devdocs/numpy_2_0_migration_guide.html#adapting-to-changes-in-the-copy-keyword.

Passing a numpy array instead avoids that code path and allows this test to pass on NumPy 2.x

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't post enough stacktrace there.

To be clear, that is code inside lightgbm: https://github.com/microsoft/LightGBM/blob/e0ac63568c900405e5e438d95c539f3490919aab/python-package/lightgbm/basic.py#L358

Copy link

Choose a reason for hiding this comment

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

The int32 is a bit unfortunate, but it, or intc is necessary here and seems totally safe to keep indefinitely.

lgb_model = lgb.train(params, dtrain, num_boost_round=1)
lgb_out = lgb_model.predict(X).reshape((-1, 1, 1))
lgb_model.save_model(lgb_model_path)
Expand Down
Loading