Skip to content

Commit

Permalink
[R-package] manage Dataset and Booster handles as R external pointers (
Browse files Browse the repository at this point in the history
…fixes #3016) (#4265)

* started converting handles

* more changes

* sort of working for Dataset

* yay all the tests are passing for Dataset handle changes

* working for other handle types

* remove debugging logging

* remove unnecessary spaces

* fix null logic

* more NULL

* updates

* Apply suggestions from code review

Co-authored-by: Nikita Titov <nekit94-08@mail.ru>

* consolidate steps

Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
  • Loading branch information
jameslamb and StrikerRUS authored May 12, 2021
1 parent 0a172d9 commit 676c95f
Show file tree
Hide file tree
Showing 9 changed files with 224 additions and 372 deletions.
11 changes: 4 additions & 7 deletions R-package/R/lgb.Booster.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Booster <- R6::R6Class(

# Create parameters and handle
params <- append(params, list(...))
handle <- lgb.null.handle()
handle <- NULL

# Attempts to create a handle for the dataset
try({
Expand All @@ -52,11 +52,10 @@ Booster <- R6::R6Class(
params <- modifyList(params, train_set$get_params())
params_str <- lgb.params2str(params = params)
# Store booster handle
.Call(
handle <- .Call(
LGBM_BoosterCreate_R
, train_set_handle
, params_str
, handle
)

# Create private booster information
Expand Down Expand Up @@ -88,10 +87,9 @@ Booster <- R6::R6Class(
}

# Create booster from model
.Call(
handle <- .Call(
LGBM_BoosterCreateFromModelfile_R
, modelfile
, handle
)

} else if (!is.null(model_str)) {
Expand All @@ -102,10 +100,9 @@ Booster <- R6::R6Class(
}

# Create booster from model
.Call(
handle <- .Call(
LGBM_BoosterLoadModelFromString_R
, model_str
, handle
)

} else {
Expand Down
13 changes: 4 additions & 9 deletions R-package/R/lgb.Dataset.R
Original file line number Diff line number Diff line change
Expand Up @@ -192,41 +192,38 @@ Dataset <- R6::R6Class(
if (!is.null(private$reference)) {
ref_handle <- private$reference$.__enclos_env__$private$get_handle()
}
handle <- lgb.null.handle()

# Not subsetting
if (is.null(private$used_indices)) {

# Are we using a data file?
if (is.character(private$raw_data)) {

.Call(
handle <- .Call(
LGBM_DatasetCreateFromFile_R
, private$raw_data
, params_str
, ref_handle
, handle
)

} else if (is.matrix(private$raw_data)) {

# Are we using a matrix?
.Call(
handle <- .Call(
LGBM_DatasetCreateFromMat_R
, private$raw_data
, nrow(private$raw_data)
, ncol(private$raw_data)
, params_str
, ref_handle
, handle
)

} else if (methods::is(private$raw_data, "dgCMatrix")) {
if (length(private$raw_data@p) > 2147483647L) {
stop("Cannot support large CSC matrix")
}
# Are we using a dgCMatrix (sparsed matrix column compressed)
.Call(
handle <- .Call(
LGBM_DatasetCreateFromCSC_R
, private$raw_data@p
, private$raw_data@i
Expand All @@ -236,7 +233,6 @@ Dataset <- R6::R6Class(
, nrow(private$raw_data)
, params_str
, ref_handle
, handle
)

} else {
Expand All @@ -257,13 +253,12 @@ Dataset <- R6::R6Class(
}

# Construct subset
.Call(
handle <- .Call(
LGBM_DatasetGetSubset_R
, ref_handle
, c(private$used_indices) # Adding c() fixes issue in R v3.5
, length(private$used_indices)
, params_str
, handle
)

}
Expand Down
6 changes: 2 additions & 4 deletions R-package/R/lgb.Predictor.R
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,15 @@ Predictor <- R6::R6Class(
initialize = function(modelfile, ...) {
params <- list(...)
private$params <- lgb.params2str(params = params)
# Create new lgb handle
handle <- lgb.null.handle()
handle <- NULL

# Check if handle is a character
if (is.character(modelfile)) {

# Create handle on it
.Call(
handle <- .Call(
LGBM_BoosterCreateFromModelfile_R
, modelfile
, handle
)
private$need_free_handle <- TRUE

Expand Down
15 changes: 6 additions & 9 deletions R-package/R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,13 @@ lgb.is.Dataset <- function(x) {
return(lgb.check.r6.class(object = x, name = "lgb.Dataset"))
}

lgb.null.handle <- function() {
if (.Machine$sizeof.pointer == 8L) {
return(NA_real_)
} else {
return(NA_integer_)
}
}

lgb.is.null.handle <- function(x) {
return(is.null(x) || is.na(x))
if (is.null(x)) {
return(TRUE)
}
return(
isTRUE(.Call(LGBM_HandleIsNull_R, x))
)
}

# [description] Get the most recent error stored on the C++ side and raise it
Expand Down
152 changes: 0 additions & 152 deletions R-package/src/R_object_helper.h

This file was deleted.

Loading

0 comments on commit 676c95f

Please sign in to comment.