Repository navigation
[c++] Preserve feature sampling on unrelated parameter updates #7458
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| /*! | ||
| * Copyright (c) 2026 The LightGBM developers. All rights reserved. | ||
| * Licensed under the MIT License. See LICENSE file in the project root for license information. | ||
| */ | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| #include <LightGBM/c_api.h> | ||
| #include <LightGBM/tree.h> | ||
|
|
||
| #include <algorithm> | ||
| #include <memory> | ||
|
|
||
| #include "../../src/treelearner/col_sampler.hpp" | ||
|
|
||
| namespace { | ||
|
|
||
| class ColSamplerTest : public testing::Test { | ||
| protected: | ||
| void SetUp() override { | ||
| double features[20 * 8]; | ||
| for (int row = 0; row < 20; ++row) { | ||
| for (int col = 0; col < 8; ++col) { | ||
| features[row * 8 + col] = row * (col + 1); | ||
| } | ||
| } | ||
| DatasetHandle handle; | ||
| ASSERT_EQ(0, LGBM_DatasetCreateFromMat(features, C_API_DTYPE_FLOAT64, 20, 8, 1, | ||
| "min_data_in_bin=1 min_data_in_leaf=1 num_threads=1", nullptr, &handle)); | ||
| data_.reset(static_cast<LightGBM::Dataset*>(handle)); | ||
| config_.feature_fraction = 0.5; | ||
| config_.feature_fraction_seed = 123; | ||
| } | ||
|
|
||
| LightGBM::Config config_; | ||
| std::unique_ptr<LightGBM::Dataset> data_; | ||
| }; | ||
|
|
||
| TEST_F(ColSamplerTest, UnrelatedConfigPreservesSampling) { | ||
| LightGBM::ColSampler control(&config_); | ||
| LightGBM::ColSampler updated(&config_); | ||
| control.SetTrainingData(data_.get()); | ||
| updated.SetTrainingData(data_.get()); | ||
| config_.learning_rate = 0.2; | ||
| updated.SetConfig(&config_); | ||
| EXPECT_EQ(control.is_feature_used_bytree(), updated.is_feature_used_bytree()); | ||
| control.ResetByTree(); | ||
| updated.ResetByTree(); | ||
| EXPECT_EQ(control.is_feature_used_bytree(), updated.is_feature_used_bytree()); | ||
| } | ||
|
|
||
| TEST_F(ColSamplerTest, SamplingConfigStillUpdates) { | ||
| LightGBM::ColSampler sampler(&config_); | ||
| sampler.SetTrainingData(data_.get()); | ||
| config_.feature_fraction = 0.25; | ||
| sampler.SetConfig(&config_); | ||
| const auto& mask = sampler.is_feature_used_bytree(); | ||
| EXPECT_EQ(2, std::count(mask.begin(), mask.end(), 1)); | ||
|
|
||
| config_.feature_fraction_bynode = 0.5; | ||
| sampler.SetConfig(&config_); | ||
| const auto node_mask = sampler.GetByNode(nullptr, 0); | ||
| EXPECT_EQ(1, std::count(node_mask.begin(), node_mask.end(), 1)); | ||
|
|
||
| config_.feature_fraction_seed = 456; | ||
| sampler.SetConfig(&config_); | ||
| LightGBM::ColSampler fresh(&config_); | ||
| fresh.SetTrainingData(data_.get()); | ||
| EXPECT_EQ(fresh.is_feature_used_bytree(), sampler.is_feature_used_bytree()); | ||
| } | ||
|
|
||
| } // namespace |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3147,6 +3147,47 @@ def test_default_objective_and_metric(): | |||||||||||||||||||||||||||
| assert len(evals_result["valid_0"]["l2"]) == 5 | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @pytest.mark.parametrize("seed", [123, None], ids=["fixed_seed", "randomized"]) | ||||||||||||||||||||||||||||
| @pytest.mark.parametrize("update", ["custom_objective", "reset_parameter"]) | ||||||||||||||||||||||||||||
| def test_feature_sampling_is_unchanged_by_unrelated_updates(seed, update): | ||||||||||||||||||||||||||||
| if seed is None: | ||||||||||||||||||||||||||||
| seed = int(np.random.default_rng().integers(0, 1000000)) | ||||||||||||||||||||||||||||
| rng = np.random.RandomState(1) | ||||||||||||||||||||||||||||
| X, y = rng.randn(300, 300), rng.randn(300) | ||||||||||||||||||||||||||||
| params = { | ||||||||||||||||||||||||||||
| "objective": "regression", | ||||||||||||||||||||||||||||
| "max_depth": 1, | ||||||||||||||||||||||||||||
| "learning_rate": 0.001, | ||||||||||||||||||||||||||||
| "feature_fraction": 0.5, | ||||||||||||||||||||||||||||
| "seed": seed, | ||||||||||||||||||||||||||||
| "deterministic": True, | ||||||||||||||||||||||||||||
| "force_row_wise": True, | ||||||||||||||||||||||||||||
| "num_threads": 1, | ||||||||||||||||||||||||||||
| "verbosity": -1, | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
| dataset = lgb.Dataset(X, label=y, init_score=np.zeros_like(y)) | ||||||||||||||||||||||||||||
| expected = lgb.train(params, dataset, num_boost_round=3) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| def custom_l2(pred, data): | ||||||||||||||||||||||||||||
| return pred - data.get_label(), np.ones_like(pred) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| def reset_same_learning_rate(env): | ||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry but I am not understanding this... how is the learning rate involved in any of this? I don't understand what the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the goal is just to set some parameter to a non-default value to be sure the initial Either way, please do add some assertions here checking that other overrides ( LightGBM/python-package/lightgbm/basic.py Lines 3854 to 3866 in 9474d04
|
||||||||||||||||||||||||||||
| if env.iteration == 0: | ||||||||||||||||||||||||||||
| env.model.reset_parameter({"learning_rate": params["learning_rate"]}) | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| reset_same_learning_rate.before_iteration = True | ||||||||||||||||||||||||||||
| if update == "custom_objective": | ||||||||||||||||||||||||||||
| params["objective"] = custom_l2 | ||||||||||||||||||||||||||||
| callbacks = None | ||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||
| callbacks = [reset_same_learning_rate] | ||||||||||||||||||||||||||||
| actual = lgb.train(params, dataset, num_boost_round=3, callbacks=callbacks) | ||||||||||||||||||||||||||||
| expected_features = [tree["tree_structure"]["split_feature"] for tree in expected.dump_model()["tree_info"]] | ||||||||||||||||||||||||||||
| actual_features = [tree["tree_structure"]["split_feature"] for tree in actual.dump_model()["tree_info"]] | ||||||||||||||||||||||||||||
| assert actual_features == expected_features, f"seed={seed}" | ||||||||||||||||||||||||||||
| np.testing.assert_allclose(actual.predict(X), expected.predict(X), err_msg=f"seed={seed}") | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| @pytest.mark.parametrize("use_weight", [True, False]) | ||||||||||||||||||||||||||||
| def test_multiclass_custom_objective(use_weight): | ||||||||||||||||||||||||||||
| def custom_obj(y_pred, ds): | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a reason to override the default for
learning_ratehere, let's remove this.And I think if you follow my other recommendations, you'll need to train deeper trees than
max_depth: 1.