From 9a311771425fbddeec10b5e4d0711d20a2d8fd00 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 20 Dec 2021 19:34:59 -0600 Subject: [PATCH 1/2] [R-package] update parameter 'verbosity' based on keyword arg 'verbose' --- R-package/R/lgb.cv.R | 6 +++++- R-package/R/lgb.train.R | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/R-package/R/lgb.cv.R b/R-package/R/lgb.cv.R index 0690936f5624..9195c33c6b8b 100644 --- a/R-package/R/lgb.cv.R +++ b/R-package/R/lgb.cv.R @@ -102,7 +102,6 @@ lgb.cv <- function(params = list() } # Setup temporary variables - params$verbose <- verbose params <- lgb.check.obj(params = params, obj = obj) params <- lgb.check.eval(params = params, eval = eval) fobj <- NULL @@ -112,6 +111,11 @@ lgb.cv <- function(params = list() # in `params`. # this ensures that the model stored with Booster$save() correctly represents # what was passed in + params <- lgb.check.wrapper_param( + main_param_name = "verbosity" + , params = params + , alternative_kwarg_value = verbose + ) params <- lgb.check.wrapper_param( main_param_name = "num_iterations" , params = params diff --git a/R-package/R/lgb.train.R b/R-package/R/lgb.train.R index b3e19c7185d2..1fd40d596d06 100644 --- a/R-package/R/lgb.train.R +++ b/R-package/R/lgb.train.R @@ -74,7 +74,6 @@ lgb.train <- function(params = list(), } # Setup temporary variables - params$verbose <- verbose params <- lgb.check.obj(params = params, obj = obj) params <- lgb.check.eval(params = params, eval = eval) fobj <- NULL @@ -84,6 +83,11 @@ lgb.train <- function(params = list(), # in `params`. # this ensures that the model stored with Booster$save() correctly represents # what was passed in + params <- lgb.check.wrapper_param( + main_param_name = "verbosity" + , params = params + , alternative_kwarg_value = verbose + ) params <- lgb.check.wrapper_param( main_param_name = "num_iterations" , params = params From 2057636a0cb6c3a16c0da2702ca1c0d45ad27418 Mon Sep 17 00:00:00 2001 From: James Lamb Date: Mon, 20 Dec 2021 20:08:40 -0600 Subject: [PATCH 2/2] add tests --- R-package/tests/testthat/test_basic.R | 122 ++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/R-package/tests/testthat/test_basic.R b/R-package/tests/testthat/test_basic.R index 6ef2c333b35d..8b2b584e8563 100644 --- a/R-package/tests/testthat/test_basic.R +++ b/R-package/tests/testthat/test_basic.R @@ -1534,6 +1534,60 @@ test_that("lgb.train() works with integer, double, and numeric data", { } }) +test_that("lgb.train() updates params based on keyword arguments", { + dtrain <- lgb.Dataset( + data = matrix(rnorm(400L), ncol = 4L) + , label = rnorm(100L) + ) + + # defaults from keyword arguments should be used if not specified in params + invisible( + capture.output({ + bst <- lgb.train( + data = dtrain + , obj = "regression" + , params = list() + ) + }) + ) + expect_equal(bst$params[["verbosity"]], 1L) + expect_equal(bst$params[["num_iterations"]], 100L) + + # main param names should be preferred to keyword arguments + invisible( + capture.output({ + bst <- lgb.train( + data = dtrain + , obj = "regression" + , params = list( + "verbosity" = 5L + , "num_iterations" = 2L + ) + ) + }) + ) + expect_equal(bst$params[["verbosity"]], 5L) + expect_equal(bst$params[["num_iterations"]], 2L) + + # aliases should be preferred to keyword arguments, and converted to main parameter name + invisible( + capture.output({ + bst <- lgb.train( + data = dtrain + , obj = "regression" + , params = list( + "verbose" = 5L + , "num_boost_round" = 2L + ) + ) + }) + ) + expect_equal(bst$params[["verbosity"]], 5L) + expect_false("verbose" %in% bst$params) + expect_equal(bst$params[["num_iterations"]], 2L) + expect_false("num_boost_round" %in% bst$params) +}) + test_that("when early stopping is not activated, best_iter and best_score come from valids and not training data", { set.seed(708L) trainDF <- data.frame( @@ -1953,6 +2007,74 @@ test_that("early stopping works with lgb.cv()", { ) }) +test_that("lgb.cv() updates params based on keyword arguments", { + dtrain <- lgb.Dataset( + data = matrix(rnorm(400L), ncol = 4L) + , label = rnorm(100L) + ) + + # defaults from keyword arguments should be used if not specified in params + invisible( + capture.output({ + cv_bst <- lgb.cv( + data = dtrain + , obj = "regression" + , params = list() + , nfold = 2L + ) + }) + ) + + for (bst in cv_bst$boosters) { + bst_params <- bst[["booster"]]$params + expect_equal(bst_params[["verbosity"]], 1L) + expect_equal(bst_params[["num_iterations"]], 100L) + } + + # main param names should be preferred to keyword arguments + invisible( + capture.output({ + cv_bst <- lgb.cv( + data = dtrain + , obj = "regression" + , params = list( + "verbosity" = 5L + , "num_iterations" = 2L + ) + , nfold = 2L + ) + }) + ) + for (bst in cv_bst$boosters) { + bst_params <- bst[["booster"]]$params + expect_equal(bst_params[["verbosity"]], 5L) + expect_equal(bst_params[["num_iterations"]], 2L) + } + + # aliases should be preferred to keyword arguments, and converted to main parameter name + invisible( + capture.output({ + cv_bst <- lgb.cv( + data = dtrain + , obj = "regression" + , params = list( + "verbose" = 5L + , "num_boost_round" = 2L + ) + , nfold = 2L + ) + }) + ) + for (bst in cv_bst$boosters) { + bst_params <- bst[["booster"]]$params + expect_equal(bst_params[["verbosity"]], 5L) + expect_false("verbose" %in% bst_params) + expect_equal(bst_params[["num_iterations"]], 2L) + expect_false("num_boost_round" %in% bst_params) + } + +}) + context("linear learner") test_that("lgb.train() fit on linearly-relatead data improves when using linear learners", {