Unverified Commit 1e95cb09 authored by James Lamb's avatar James Lamb Committed by GitHub
Browse files

[R-package] Move error handling into C++ side (#4163)

* [R-package] raise errors from C++ side

* working but a lot of warnings

* more changes

* simplify

* cleanup

* linting

* fix valgrind issues

* revert unnecessary change
parent 0a847efe
...@@ -545,7 +545,9 @@ Dataset <- R6::R6Class( ...@@ -545,7 +545,9 @@ Dataset <- R6::R6Class(
}, },
# Update parameters # [description] Update Dataset parameters. If it has not been constructed yet,
# this operation just happens on the R side (updating private$params).
# If it has been constructed, parameters will be updated on the C++ side.
update_params = function(params) { update_params = function(params) {
if (length(params) == 0L) { if (length(params) == 0L) {
return(invisible(self)) return(invisible(self))
...@@ -553,26 +555,27 @@ Dataset <- R6::R6Class( ...@@ -553,26 +555,27 @@ Dataset <- R6::R6Class(
if (lgb.is.null.handle(x = private$handle)) { if (lgb.is.null.handle(x = private$handle)) {
private$params <- modifyList(private$params, params) private$params <- modifyList(private$params, params)
} else { } else {
tryCatch({
call_state <- 0L call_state <- 0L
call_state <- .Call( .Call(
"LGBM_DatasetUpdateParamChecking_R" "LGBM_DatasetUpdateParamChecking_R"
, lgb.params2str(params = private$params) , lgb.params2str(params = private$params)
, lgb.params2str(params = params) , lgb.params2str(params = params)
, call_state , call_state
, PACKAGE = "lib_lightgbm" , PACKAGE = "lib_lightgbm"
) )
call_state <- as.integer(call_state) }, error = function(e) {
if (call_state != 0L) { # If updating failed but raw data is not available, raise an error because
# achieving what the user asked for is not possible
# raise error if raw data is freed
if (is.null(private$raw_data)) { if (is.null(private$raw_data)) {
lgb.last_error() stop(e)
} }
# Overwrite paramms # If updating failed but raw data is available, modify the params
# on the R side and re-set ("deconstruct") the Dataset
private$params <- modifyList(private$params, params) private$params <- modifyList(private$params, params)
self$finalize() self$finalize()
} })
} }
return(invisible(self)) return(invisible(self))
......
...@@ -80,11 +80,6 @@ lgb.call <- function(fun_name, ret, ...) { ...@@ -80,11 +80,6 @@ lgb.call <- function(fun_name, ret, ...) {
, PACKAGE = "lib_lightgbm" , PACKAGE = "lib_lightgbm"
) )
} }
call_state <- as.integer(call_state)
# Check for call state value post call
if (call_state != 0L) {
lgb.last_error()
}
return(ret) return(ret)
......
...@@ -12,6 +12,10 @@ ...@@ -12,6 +12,10 @@
#include <R_ext/Rdynload.h> #include <R_ext/Rdynload.h>
#define R_NO_REMAP
#define R_USE_C99_IN_CXX
#include <R_ext/Error.h>
#include <string> #include <string>
#include <cstdio> #include <cstdio>
#include <cstring> #include <cstring>
...@@ -31,7 +35,7 @@ ...@@ -31,7 +35,7 @@
#define CHECK_CALL(x) \ #define CHECK_CALL(x) \
if ((x) != 0) { \ if ((x) != 0) { \
R_INT_PTR(call_state)[0] = -1;\ Rf_error(LGBM_GetLastError()); \
return call_state;\ return call_state;\
} }
......
...@@ -89,6 +89,19 @@ test_that("lgb.Dataset: Dataset should be able to construct from matrix and retu ...@@ -89,6 +89,19 @@ test_that("lgb.Dataset: Dataset should be able to construct from matrix and retu
handle <- NULL handle <- NULL
}) })
test_that("cpp errors should be raised as proper R errors", {
data(agaricus.train, package = "lightgbm")
train <- agaricus.train
dtrain <- lgb.Dataset(
train$data
, label = train$label
, init_score = seq_len(10L)
)
expect_error({
dtrain$construct()
}, regexp = "Initial score size doesn't match data size")
})
test_that("lgb.Dataset$setinfo() should convert 'group' to integer", { test_that("lgb.Dataset$setinfo() should convert 'group' to integer", {
ds <- lgb.Dataset( ds <- lgb.Dataset(
data = matrix(rnorm(100L), nrow = 50L, ncol = 2L) data = matrix(rnorm(100L), nrow = 50L, ncol = 2L)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment