From 39f0961b14543c7fa3674bb05cbf0308efc9c45f Mon Sep 17 00:00:00 2001 From: Guolin Ke Date: Sat, 15 Aug 2020 10:20:34 +0800 Subject: [PATCH 1/3] use integer type for handle --- R-package/R/lgb.Booster.R | 2 +- R-package/R/lgb.Dataset.R | 2 +- R-package/tests/testthat/test_dataset.R | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R-package/R/lgb.Booster.R b/R-package/R/lgb.Booster.R index 867450eabed0..b701825ba5d5 100644 --- a/R-package/R/lgb.Booster.R +++ b/R-package/R/lgb.Booster.R @@ -31,7 +31,7 @@ Booster <- R6::R6Class( # Create parameters and handle params <- append(params, list(...)) - handle <- 0.0 + handle <- 0L # Attempts to create a handle for the dataset try({ diff --git a/R-package/R/lgb.Dataset.R b/R-package/R/lgb.Dataset.R index 9bfe0af21221..1526d2c0046d 100644 --- a/R-package/R/lgb.Dataset.R +++ b/R-package/R/lgb.Dataset.R @@ -186,7 +186,7 @@ Dataset <- R6::R6Class( if (!is.null(private$reference)) { ref_handle <- private$reference$.__enclos_env__$private$get_handle() } - handle <- NA_real_ + handle <- 0L # Not subsetting if (is.null(private$used_indices)) { diff --git a/R-package/tests/testthat/test_dataset.R b/R-package/tests/testthat/test_dataset.R index f8e26b269d1e..cf0904fce395 100644 --- a/R-package/tests/testthat/test_dataset.R +++ b/R-package/tests/testthat/test_dataset.R @@ -73,7 +73,7 @@ test_that("lgb.Dataset: nrow is correct for a very sparse matrix", { test_that("lgb.Dataset: Dataset should be able to construct from matrix and return non-null handle", { rawData <- matrix(runif(1000L), ncol = 10L) - handle <- NA_real_ + handle <- 0L ref_handle <- NULL handle <- lightgbm:::lgb.call( "LGBM_DatasetCreateFromMat_R" From 01568657d666e1dc6cbf7b3bdb62b67843d4bc7a Mon Sep 17 00:00:00 2001 From: Guolin Ke Date: Sat, 15 Aug 2020 10:29:30 +0800 Subject: [PATCH 2/3] Apply suggestions from code review --- R-package/R/lgb.Booster.R | 2 +- R-package/R/lgb.Dataset.R | 2 +- R-package/tests/testthat/test_dataset.R | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/R-package/R/lgb.Booster.R b/R-package/R/lgb.Booster.R index b701825ba5d5..fb786448cc5d 100644 --- a/R-package/R/lgb.Booster.R +++ b/R-package/R/lgb.Booster.R @@ -31,7 +31,7 @@ Booster <- R6::R6Class( # Create parameters and handle params <- append(params, list(...)) - handle <- 0L + handle <- NA_integer_ # Attempts to create a handle for the dataset try({ diff --git a/R-package/R/lgb.Dataset.R b/R-package/R/lgb.Dataset.R index 1526d2c0046d..b8cd4deaee19 100644 --- a/R-package/R/lgb.Dataset.R +++ b/R-package/R/lgb.Dataset.R @@ -186,7 +186,7 @@ Dataset <- R6::R6Class( if (!is.null(private$reference)) { ref_handle <- private$reference$.__enclos_env__$private$get_handle() } - handle <- 0L + handle <- NA_integer_ # Not subsetting if (is.null(private$used_indices)) { diff --git a/R-package/tests/testthat/test_dataset.R b/R-package/tests/testthat/test_dataset.R index cf0904fce395..5df00f3e3f93 100644 --- a/R-package/tests/testthat/test_dataset.R +++ b/R-package/tests/testthat/test_dataset.R @@ -73,7 +73,7 @@ test_that("lgb.Dataset: nrow is correct for a very sparse matrix", { test_that("lgb.Dataset: Dataset should be able to construct from matrix and return non-null handle", { rawData <- matrix(runif(1000L), ncol = 10L) - handle <- 0L + handle <- NA_integer_ ref_handle <- NULL handle <- lightgbm:::lgb.call( "LGBM_DatasetCreateFromMat_R" From 846076b12f2ccf23d0f11d21242125c7cbc0f72f Mon Sep 17 00:00:00 2001 From: Guolin Ke Date: Sat, 15 Aug 2020 10:51:36 +0800 Subject: [PATCH 3/3] fix 64bit handle --- R-package/R/lgb.Booster.R | 2 +- R-package/R/lgb.Dataset.R | 2 +- R-package/R/utils.R | 8 ++++++++ R-package/tests/testthat/test_dataset.R | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/R-package/R/lgb.Booster.R b/R-package/R/lgb.Booster.R index fb786448cc5d..8765214ddcb6 100644 --- a/R-package/R/lgb.Booster.R +++ b/R-package/R/lgb.Booster.R @@ -31,7 +31,7 @@ Booster <- R6::R6Class( # Create parameters and handle params <- append(params, list(...)) - handle <- NA_integer_ + handle <- lgb.null.handle() # Attempts to create a handle for the dataset try({ diff --git a/R-package/R/lgb.Dataset.R b/R-package/R/lgb.Dataset.R index b8cd4deaee19..d531bd5fa236 100644 --- a/R-package/R/lgb.Dataset.R +++ b/R-package/R/lgb.Dataset.R @@ -186,7 +186,7 @@ Dataset <- R6::R6Class( if (!is.null(private$reference)) { ref_handle <- private$reference$.__enclos_env__$private$get_handle() } - handle <- NA_integer_ + handle <- lgb.null.handle() # Not subsetting if (is.null(private$used_indices)) { diff --git a/R-package/R/utils.R b/R-package/R/utils.R index 7373386f7bcc..d961cce8a9ab 100644 --- a/R-package/R/utils.R +++ b/R-package/R/utils.R @@ -6,6 +6,14 @@ lgb.is.Dataset <- function(x) { lgb.check.r6.class(x, "lgb.Dataset") # Checking if it is of class lgb.Dataset or not } +lgb.null.handle <- function() { + if (.Machine$sizeof.pointer == 8L) { + return(NA_real_) + } else { + return(NA_integer_) + } +} + lgb.is.null.handle <- function(x) { is.null(x) || is.na(x) } diff --git a/R-package/tests/testthat/test_dataset.R b/R-package/tests/testthat/test_dataset.R index 5df00f3e3f93..9431cb32f646 100644 --- a/R-package/tests/testthat/test_dataset.R +++ b/R-package/tests/testthat/test_dataset.R @@ -73,7 +73,7 @@ test_that("lgb.Dataset: nrow is correct for a very sparse matrix", { test_that("lgb.Dataset: Dataset should be able to construct from matrix and return non-null handle", { rawData <- matrix(runif(1000L), ncol = 10L) - handle <- NA_integer_ + handle <- lgb.null.handle() ref_handle <- NULL handle <- lightgbm:::lgb.call( "LGBM_DatasetCreateFromMat_R"