[R-package] remove behavior where lightgbm() saves model to disk (#4974)

* do not produce model files on disk by default * remove 'save_name' argument from 'lightgbm()'
microsoft · Feb 16, 2022 · 9259a53 · 9259a53
1 parent 057ba07
commit 9259a53
Show file tree

Hide file tree

Showing 7 changed files with 1 addition and 61 deletions.
diff --git a/R-package/R/lgb.restore_handle.R b/R-package/R/lgb.restore_handle.R
@@ -17,7 +17,6 @@
 #'   , agaricus.train$label
 #'   , params = list(objective = "binary", nthreads = 1L)
 #'   , nrounds = 5L
-#'   , save_name = NULL
 #'   , verbose = 0)
 #' fname <- tempfile(fileext="rds")
 #' saveRDS(model, fname)

diff --git a/R-package/R/lightgbm.R b/R-package/R/lightgbm.R
@@ -53,7 +53,7 @@
 #'               the "Parameters" section of the documentation} for a list of parameters and valid values.
 #' @param verbose verbosity for output, if <= 0, also will disable the print of evaluation during training
 #' @param serializable whether to make the resulting objects serializable through functions such as
-#' \code{save} or \code{saveRDS} (see section "Model serialization").
+#'                     \code{save} or \code{saveRDS} (see section "Model serialization").
 #' @section Early Stopping:
 #'
 #'          "early stopping" refers to stopping the training process if the model's performance on a given
@@ -92,8 +92,6 @@ NULL
 #' @inheritParams lgb_shared_params
 #' @param label Vector of labels, used if \code{data} is not an \code{\link{lgb.Dataset}}
 #' @param weight vector of response values. If not NULL, will set to dataset
-#' @param save_name File name to use when writing the trained model to disk. Should end in ".model".
-#'                  If passing `NULL`, will not save the trained model to disk.
 #' @param ... Additional arguments passed to \code{\link{lgb.train}}. For example
 #'     \itemize{
 #'        \item{\code{valids}: a list of \code{lgb.Dataset} objects, used for validation}
@@ -120,7 +118,6 @@ lightgbm <- function(data,
                      verbose = 1L,
                      eval_freq = 1L,
                      early_stopping_rounds = NULL,
-                     save_name = "lightgbm.model",
                      init_model = NULL,
                      callbacks = list(),
                      serializable = TRUE,
@@ -167,11 +164,6 @@ lightgbm <- function(data,
     , args = train_args
   )
 
-  # Store model under a specific name
-  if (!is.null(save_name)) {
-    bst$save_model(filename = save_name)
-  }
-
   return(bst)
 }
 

diff --git a/R-package/man/lgb.restore_handle.Rd b/R-package/man/lgb.restore_handle.Rd
diff --git a/R-package/man/lightgbm.Rd b/R-package/man/lightgbm.Rd
diff --git a/R-package/tests/testthat/test_basic.R b/R-package/tests/testthat/test_basic.R
@@ -80,7 +80,6 @@ test_that("train and predict binary classification", {
         , metric = "binary_error"
     )
     , nrounds = nrounds
-    , save_name = tempfile(fileext = ".model")
   )
   expect_false(is.null(bst$record_evals))
   record_results <- lgb.get.eval.result(bst, "train", "binary_error")
@@ -114,7 +113,6 @@ test_that("train and predict softmax", {
         , num_class = 3L
     )
     , nrounds = 20L
-    , save_name = tempfile(fileext = ".model")
   )
 
   expect_false(is.null(bst$record_evals))
@@ -138,7 +136,6 @@ test_that("use of multiple eval metrics works", {
         , metric = metrics
     )
     , nrounds = 10L
-    , save_name = tempfile(fileext = ".model")
   )
   expect_false(is.null(bst$record_evals))
   expect_named(
@@ -161,7 +158,6 @@ test_that("lgb.Booster.upper_bound() and lgb.Booster.lower_bound() work as expec
         , metric = "binary_error"
     )
     , nrounds = nrounds
-    , save_name = tempfile(fileext = ".model")
   )
   expect_true(abs(bst$lower_bound() - -1.590853) < TOLERANCE)
   expect_true(abs(bst$upper_bound() - 1.871015) <  TOLERANCE)
@@ -179,7 +175,6 @@ test_that("lgb.Booster.upper_bound() and lgb.Booster.lower_bound() work as expec
         , metric = "l2"
     )
     , nrounds = nrounds
-    , save_name = tempfile(fileext = ".model")
   )
   expect_true(abs(bst$lower_bound() - 0.1513859) < TOLERANCE)
   expect_true(abs(bst$upper_bound() - 0.9080349) < TOLERANCE)
@@ -194,7 +189,6 @@ test_that("lightgbm() rejects negative or 0 value passed to nrounds", {
         data = dtrain
         , params = params
         , nrounds = nround_value
-        , save_name = tempfile(fileext = ".model")
       )
     }, "nrounds should be greater than zero")
   }
@@ -213,7 +207,6 @@ test_that("lightgbm() accepts nrounds as either a top-level argument or paramete
       , metric = "l2"
       , num_leaves = 5L
     )
-    , save_name = tempfile(fileext = ".model")
   )
 
   set.seed(708L)
@@ -226,7 +219,6 @@ test_that("lightgbm() accepts nrounds as either a top-level argument or paramete
       , num_leaves = 5L
       , nrounds = nrounds
     )
-    , save_name = tempfile(fileext = ".model")
   )
 
   set.seed(708L)
@@ -240,7 +232,6 @@ test_that("lightgbm() accepts nrounds as either a top-level argument or paramete
       , num_leaves = 5L
       , nrounds = nrounds
     )
-    , save_name = tempfile(fileext = ".model")
   )
 
   top_level_l2 <- top_level_bst$eval_train()[[1L]][["value"]]
@@ -289,7 +280,6 @@ test_that("lightgbm() performs evaluation on validation sets if they are provide
       "valid1" = dvalid1
       , "valid2" = dvalid2
     )
-    , save_name = tempfile(fileext = ".model")
   )
 
   expect_named(
@@ -307,23 +297,6 @@ test_that("lightgbm() performs evaluation on validation sets if they are provide
   expect_true(abs(bst$record_evals[["valid2"]][["binary_error"]][["eval"]][[1L]] - 0.02226317) < TOLERANCE)
 })
 
-test_that("lightgbm() does not write model to disk if save_name=NULL", {
-  files_before <- list.files(getwd())
-
-  model <- lightgbm(
-    data = train$data
-    , label = train$label
-    , nrounds = 5L
-    , params = list(objective = "binary")
-    , verbose = 0L
-    , save_name = NULL
-  )
-
-  files_after <- list.files(getwd())
-
-  expect_equal(files_before, files_after)
-})
-
 test_that("training continuation works", {
   dtrain <- lgb.Dataset(
     train$data
@@ -742,7 +715,6 @@ test_that("lgb.train() accepts nrounds as either a top-level argument or paramet
       objective = "regression"
       , metric = "l2"
       , num_leaves = 5L
-      , save_name = tempfile(fileext = ".model")
       , verbose = VERBOSITY
     )
   )
@@ -758,7 +730,6 @@ test_that("lgb.train() accepts nrounds as either a top-level argument or paramet
       , metric = "l2"
       , num_leaves = 5L
       , nrounds = nrounds
-      , save_name = tempfile(fileext = ".model")
       , verbose = VERBOSITY
     )
   )
@@ -775,7 +746,6 @@ test_that("lgb.train() accepts nrounds as either a top-level argument or paramet
       , metric = "l2"
       , num_leaves = 5L
       , nrounds = nrounds
-      , save_name = tempfile(fileext = ".model")
       , verbose = VERBOSITY
     )
   )
@@ -1981,7 +1951,6 @@ test_that("using lightgbm() without early stopping, best_iter and best_score com
       , num_leaves = 5L
     )
     , verbose = -7L
-    , save_name = tempfile(fileext = ".model")
   )
   # when verbose <= 0 is passed to lightgbm(), 'valids' is passed through to lgb.train()
   # untouched. If you set verbose to > 0, the training data will still be first but called "train"

diff --git a/R-package/tests/testthat/test_lgb.Booster.R b/R-package/tests/testthat/test_lgb.Booster.R
@@ -136,7 +136,6 @@ test_that("lgb.load() gives the expected error messages given different incorrec
             , verbose = VERBOSITY
         )
         , nrounds = 2L
-        , save_name = tempfile(fileext = ".model")
     )
 
     # you have to give model_str or filename
@@ -183,7 +182,6 @@ test_that("Loading a Booster from a text file works", {
             , verbose = VERBOSITY
         )
         , nrounds = 2L
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst))
 
@@ -260,7 +258,6 @@ test_that("Loading a Booster from a string works", {
             , verbose = VERBOSITY
         )
         , nrounds = 2L
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst))
 
@@ -292,7 +289,6 @@ test_that("Saving a large model to string should work", {
             , objective = "binary"
         )
         , nrounds = 500L
-        , save_name = tempfile(fileext = ".model")
         , verbose = VERBOSITY
     )
 
@@ -336,7 +332,6 @@ test_that("Saving a large model to JSON should work", {
             , objective = "binary"
         )
         , nrounds = 200L
-        , save_name = tempfile(fileext = ".model")
         , verbose = VERBOSITY
     )
 
@@ -367,7 +362,6 @@ test_that("If a string and a file are both passed to lgb.load() the file is used
             , verbose = VERBOSITY
         )
         , nrounds = 2L
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst))
 
@@ -423,7 +417,6 @@ test_that("Creating a Booster from a Dataset with an existing predictor should w
             , verbose = VERBOSITY
         )
         , nrounds = nrounds
-        , save_name = tempfile(fileext = ".model")
     )
     data(agaricus.test, package = "lightgbm")
     dtest <- Dataset$new(
@@ -517,7 +510,6 @@ test_that("Booster$rollback_one_iter() should work as expected", {
             , verbose = VERBOSITY
         )
         , nrounds = nrounds
-        , save_name = tempfile(fileext = ".model")
     )
     expect_equal(bst$current_iter(), nrounds)
     expect_true(lgb.is.Booster(bst))
@@ -552,7 +544,6 @@ test_that("Booster$update() passing a train_set works as expected", {
             , verbose = VERBOSITY
         )
         , nrounds = nrounds
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst))
     expect_equal(bst$current_iter(), nrounds)
@@ -576,7 +567,6 @@ test_that("Booster$update() passing a train_set works as expected", {
             , verbose = VERBOSITY
         )
         , nrounds = nrounds +  1L
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst2))
     expect_equal(bst2$current_iter(), nrounds +  1L)
@@ -602,7 +592,6 @@ test_that("Booster$update() throws an informative error if you provide a non-Dat
             , verbose = VERBOSITY
         )
         , nrounds = nrounds
-        , save_name = tempfile(fileext = ".model")
     )
     expect_error({
         bst$update(
@@ -698,7 +687,6 @@ test_that("Saving a model with different feature importance types works", {
             , verbose = VERBOSITY
         )
         , nrounds = 2L
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst))
 
@@ -754,7 +742,6 @@ test_that("Saving a model with unknown importance type fails", {
             , verbose = VERBOSITY
         )
         , nrounds = 2L
-        , save_name = tempfile(fileext = ".model")
     )
     expect_true(lgb.is.Booster(bst))
 

diff --git a/R-package/tests/testthat/test_parameters.R b/R-package/tests/testthat/test_parameters.R
@@ -23,7 +23,6 @@ test_that("Feature penalties work properly", {
       )
       , nrounds = 5L
       , verbose = -1L
-      , save_name = tempfile(fileext = ".model")
     )
   })
 
@@ -75,7 +74,6 @@ test_that("training should warn if you use 'dart' boosting, specified with 'boos
         , params = params
         , nrounds = 5L
         , verbose = -1L
-        , save_name = tempfile(fileext = ".model")
       )
     }, regexp = "Early stopping is not available in 'dart' mode")
   }