[R-package] Printing a booster trained with lgb.train() errors unexpectedly

[walkerjameschris]

Description

It is possible to train a model using lgb.train() without specifying an objective in the list of params. However, if you try to print the booster it will fail. This is because print.lgb.Booster() checks if obj == "none" which does not play well with NULL.

library(lightgbm)

ds <-lgb.Dataset(
  data = as.matrix(mtcars[, -1])
  , label = mtcars[, 1]
)

lgb.train(
  # Empty params
  params = list()
  , data = ds
  , verbose = -1
)
#> Error in `if (obj == "none") ...`:
#> ! argument is of length zero
#> Show Traceback

The simplest thing is just to handle the possibility of NULL in print.lgb.Booster(), but it could also be addressed in lgb.train() upstream.

LightGBM/R-package/R/lgb.Booster.R

Lines 1236 to 1240 in 6b624fb

	if (!handle_is_null) {
	obj <- x$params$objective
	if (obj == "none") {
	obj <- "custom"
	}

Happy to make a PR if you want to go in a specific direction.

[jameslamb]

Thanks for the report! We'd welcome a PR to fix this.

it could also be addressed in lgb.train() upstream

I'm not sure exactly what you're envisioning, but I would not want to see something like this added in lgb.train:

if (is.null(params$objective)) {
    params$objective <- "regression"
}

I'd be opposed to that because it duplicates logic that is already in the core LightGBM C/C++ library. That's already leaked a bit into the R package, e.g. here:

LightGBM/R-package/R/lgb.DataProcessor.R

Lines 65 to 67 in 6b624fb

	if (objective == "auto") {
	objective <- "regression"
	}

The params object in LightGBM's interface isn't really like "the full state of all configuration"... it should be thought of as "overrides of LightGBM's default configuration".

If you're familiar with REST APIs, the params object is like the body you'd attach with a PATCH request, not the one you'd attached with a POST / PUT request.

We've tried to explain that here: https://lightgbm.readthedocs.io/en/latest/Parameters.html#parameters-format

So given all that.... if I've understood correctly what you mean by "addressed in lgb.train()", then I think a PR just updating print.lgb.Booster() (and covering print(), show(), and summary() with tests) would be better. You can see the existing tests on those methods for reference:

LightGBM/R-package/tests/testthat/test_lgb.Booster.R

Line 1521 in 6b624fb

test_that("Booster's print, show, and summary work correctly", {

I'd support pulling the internal functions up out of that one test case and re-using it across multiple test cases.

[walkerjameschris]

I’ll take a look and get started soon!