Skip to content

[python-package] Added tests on Booster.shuffle_model() #7168

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

Open
daguirre11 wants to merge 1 commit into
base: main
Choose a base branch
from
+20 −0
Open

[python-package] Added tests on Booster.shuffle_model() #7168

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions tests/python_package_test/test_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1098,3 +1098,23 @@ def test_set_field_none_removes_field(rng, field_name):

d.set_field(field_name, None)
assert d.get_field(field_name) is None


def test_booster_shuffle_models() -> None:
X_train, _, y_train, _ = train_test_split(
*load_breast_cancer(return_X_y=True),
test_size=0.1,
random_state=42,
)
Comment on lines +1104 to +1108

@jameslamb jameslamb Mar 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
X_train, _, y_train, _ = train_test_split(
*load_breast_cancer(return_X_y=True),
test_size=0.1,
random_state=42,
)
X, y = load_breast_cancer(return_X_y=True)

The test isn't using the held-out validation data, let's skip the unnecessary train-test splitting.

@daguirre11 daguirre11 Mar 23, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jameslamb sorry for the late response, I will address all your responses on PRs today. Thank you for reviewing!

@jameslamb jameslamb May 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👋🏻 hey @daguirre11 , will you have some time soon to return to these PRs? I'd be happy to help if you have any questions.

Thanks for all your efforts so far, I really appreciate it!

train_set = lgb.Dataset(X_train, label=y_train)
booster = lgb.Booster(
params={"objective": "binary", "verbose": -1},
train_set=train_set,
)
for _ in range(2):
booster.update()
Comment on lines +1109 to +1115

@jameslamb jameslamb Mar 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
train_set = lgb.Dataset(X_train, label=y_train)
booster = lgb.Booster(
params={"objective": "binary", "verbose": -1},
train_set=train_set,
)
for _ in range(2):
booster.update()
booster = lgb.train(
params={
"objective": "binary",
"num_iterations": 10,
"num_leaves": 7,
"verbose": -1,
},
train_set=lgb.Dataset(X, label=y),
)

Let's use lgb.train() for this instead of a for loop and an update please, and let's make the model smaller so the test is faster.


model_str_before = booster.model_to_string()
booster.shuffle_models(start_iteration=0, end_iteration=-1)
model_str_after = booster.model_to_string()
assert model_str_before != model_str_after

@jameslamb jameslamb Mar 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not a very strong test. For example, it'd pass if shuffle_models() corrupted the model in some serious and incorrect way. This should be made much stricter.

To do that, you'll have to look a bit deeper into what the function is doing. Start with the docstring:

LightGBM/python-package/lightgbm/basic.py

Lines 4509 to 4515 in e3d5270

Parameters
----------
start_iteration : int, optional (default=0)
The first iteration that will be shuffled.
end_iteration : int, optional (default=-1)
The last iteration that will be shuffled.
If <= 0, means the last available iteration.

The test should train 10 trees (for example) and:

  1. omit the first 2 trees, and confirm that their placement is not changed
  2. omit the final tree, and confirm that its placement isn't changed
  3. confirm that the set of trees is identical and only the ordering is different
  4. confirm that booster.predict() (with start_iteration left at its default) produces identical results before and after (ordering should not affect the predictions if you predict with all trees)
  5. check that the expected behavior happens if start_iteration is negative or end_iteration is larger than the number of trees in the model

Loading