fix force_col_wise / force_row_wise tests again

microsoft · Sep 7, 2023 · 4c98183 · 4c98183
1 parent ea4c26c
commit 4c98183
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 18 deletions.
diff --git a/R-package/tests/testthat/test_lgb.Booster.R b/R-package/tests/testthat/test_lgb.Booster.R
@@ -806,7 +806,6 @@ test_that("all parameters are stored correctly with save_model_to_string()", {
             , num_threads = .LGB_MAX_THREADS
             , seed = 708L
             , data_sample_strategy = "bagging"
-            , force_col_wise = TRUE
             , sub_row = 0.8234
         )
         , data = dtrain
@@ -821,14 +820,6 @@ test_that("all parameters are stored correctly with save_model_to_string()", {
         , "[metric: l2,l1]"
         , "[data_sample_strategy: bagging]"
         , "[seed: 708]"
-        # if force_col_wise / force_row_wise aren't explicitly set, they'll be chosen based on timing tests
-        # at Dataset construction time... setting them explicitly makes this test always pass
-        #
-        # force_col_wise is used here to avoid another if-else branch in the test... LightGBM
-        # sets force_col_wise=True and force_row_wise=False automatically for the GPU and CUDA builds
-        # https://github.com/microsoft/LightGBM/blob/1d7ee63686272bceffd522284127573b511df6be/src/io/config.cpp#L375-L377
-        , "[force_col_wise: 1]"
-        , "[force_row_wise: 0]"
         # this was passed in with alias 'sub_row'
         , "[bagging_fraction: 0.8234]"
         , "[num_iterations: 3]"
@@ -884,6 +875,8 @@ test_that("all parameters are stored correctly with save_model_to_string()", {
         , "[monotone_penalty: 0]"
         , "[feature_contri: ]"
         , "[forcedsplits_filename: ]"
+        , "[force_col_wise: 0]"
+        , "[force_row_wise: 0]"
         , "[refit_decay_rate: 0.9]"
         , "[cegb_tradeoff: 1]"
         , "[cegb_penalty_split: 0]"

diff --git a/tests/python_package_test/test_engine.py b/tests/python_package_test/test_engine.py
@@ -1514,7 +1514,6 @@ def test_all_expected_params_are_written_out_to_model_text(tmp_path):
         'metric': ['l2', 'mae'],
         'seed': 708,
         'data_sample_strategy': 'bagging',
-        'force_col_wise': True,
         'sub_row': 0.8234,
         'verbose': -1
     }
@@ -1540,14 +1539,6 @@ def test_all_expected_params_are_written_out_to_model_text(tmp_path):
         "[metric: l2,l1]",
         "[data_sample_strategy: bagging]",
         "[seed: 708]",
-        # if force_col_wise / force_row_wise aren't explicitly set, they'll be chosen based on timing tests
-        # at Dataset construction time... setting them explicitly makes this test always pass
-        #
-        # force_col_wise is used here to avoid another if-else branch in the test... LightGBM
-        # sets force_col_wise=True and force_row_wise=False automatically for the GPU and CUDA builds
-        # https://github.com/microsoft/LightGBM/blob/1d7ee63686272bceffd522284127573b511df6be/src/io/config.cpp#L375-L377
-        "[force_col_wise: 1]",
-        "[force_row_wise: 0]",
         # NOTE: this was passed in with alias 'sub_row'
         "[bagging_fraction: 0.8234]",
         "[num_iterations: 3]",
@@ -1667,18 +1658,27 @@ def test_all_expected_params_are_written_out_to_model_text(tmp_path):
     all_param_entries = non_default_param_entries + default_param_entries
 
     # add device-specific entries
+    #
+    # passed-in force_col_wise / force_row_wise parameters are ignored on CUDA and GPU builds...
+    # https://github.com/microsoft/LightGBM/blob/1d7ee63686272bceffd522284127573b511df6be/src/io/config.cpp#L375-L377
     if getenv('TASK', '') == 'cuda':
         device_entries = [
+            "[force_col_wise: 0]",
+            "[force_row_wise: 1]",
             "[device_type: cuda]",
             "[gpu_use_dp: 1]"
         ]
     elif getenv('TASK', '') == 'gpu':
         device_entries = [
+            "[force_col_wise: 1]",
+            "[force_row_wise: 0]",
             "[device_type: gpu]",
             "[gpu_use_dp: 0]"
         ]
     else:
         device_entries = [
+            "[force_col_wise: 0]",
+            "[force_row_wise: 0]",
             "[device_type: cpu]",
             "[gpu_use_dp: 0]"
         ]