diff --git a/src/mlpack/bindings/R/mlpack/src/r_util.cpp b/src/mlpack/bindings/R/mlpack/src/r_util.cpp index e529fcdd75..b8be116ae8 100644 --- a/src/mlpack/bindings/R/mlpack/src/r_util.cpp +++ b/src/mlpack/bindings/R/mlpack/src/r_util.cpp @@ -35,7 +35,6 @@ bool inline inplace_transpose(arma::Mat& X) SEXP CreateParams(const std::string& bindingName) { util::Params* p = new util::Params(IO::Parameters(bindingName)); - std::cout << "create params " << p << "\n"; return std::move(Rcpp::XPtr(p)); } @@ -44,7 +43,6 @@ SEXP CreateParams(const std::string& bindingName) SEXP CreateTimers() { util::Timers* t = new util::Timers(); - std::cout << "create timers " << t << "\n"; return std::move(Rcpp::XPtr(t)); } @@ -91,8 +89,8 @@ void SetParamBool(SEXP params, const std::string& paramName, bool paramValue) // Call params.Get>() to set the value of a parameter. // [[Rcpp::export]] void SetParamVecString(SEXP params, - const std::string& paramName, - const std::vector& str) + const std::string& paramName, + const std::vector& str) { util::Params& p = *Rcpp::as>(params); p.Get>(paramName) = std::move(str); @@ -102,8 +100,8 @@ void SetParamVecString(SEXP params, // Call params.Get>() to set the value of a parameter. // [[Rcpp::export]] void SetParamVecInt(SEXP params, - const std::string& paramName, - const std::vector& ints) + const std::string& paramName, + const std::vector& ints) { util::Params& p = *Rcpp::as>(params); p.Get>(paramName) = std::move(ints); @@ -113,8 +111,8 @@ void SetParamVecInt(SEXP params, // Call params.Get() to set the value of a parameter. // [[Rcpp::export]] void SetParamMat(SEXP params, - const std::string& paramName, - const arma::mat& paramValue) + const std::string& paramName, + const arma::mat& paramValue) { util::Params& p = *Rcpp::as>(params); p.Get(paramName) = paramValue.t(); @@ -124,8 +122,8 @@ void SetParamMat(SEXP params, // Call params.Get>() to set the value of a parameter. // [[Rcpp::export]] void SetParamUMat(SEXP params, - const std::string& paramName, - const arma::Mat& paramValue) + const std::string& paramName, + const arma::Mat& paramValue) { util::Params& p = *Rcpp::as>(params); p.Get>(paramName) = paramValue.t(); @@ -135,8 +133,8 @@ void SetParamUMat(SEXP params, // Call params.Get() to set the value of a parameter. // [[Rcpp::export]] void SetParamRow(SEXP params, - const std::string& paramName, - const arma::rowvec& paramValue) + const std::string& paramName, + const arma::rowvec& paramValue) { util::Params& p = *Rcpp::as>(params); p.Get(paramName) = std::move(paramValue); @@ -146,10 +144,19 @@ void SetParamRow(SEXP params, // Call params.Get>() to set the value of a parameter. // [[Rcpp::export]] void SetParamURow(SEXP params, - const std::string& paramName, - const arma::Row& paramValue) + const std::string& paramName, + const arma::Row& paramValue) { util::Params& p = *Rcpp::as>(params); + + // Check for zeros in the input---if we received these, the user is mistaken, + // because in R labels should start from 1. + if (arma::any(paramValue == 0)) + { + Log::Fatal << "When passing labels from R to mlpack, labels should be in " + << "the range from 1 to the number of classes!" << std::endl; + } + p.Get>(paramName) = paramValue - 1; p.SetPassed(paramName); } @@ -157,8 +164,8 @@ void SetParamURow(SEXP params, // Call params.Get() to set the value of a parameter. // [[Rcpp::export]] void SetParamCol(SEXP params, - const std::string& paramName, - const arma::vec& paramValue) + const std::string& paramName, + const arma::vec& paramValue) { util::Params& p = *Rcpp::as>(params); p.Get(paramName) = std::move(paramValue); @@ -168,10 +175,19 @@ void SetParamCol(SEXP params, // Call params.Get>() to set the value of a parameter. // [[Rcpp::export]] void SetParamUCol(SEXP params, - const std::string& paramName, - const arma::Col& paramValue) + const std::string& paramName, + const arma::Col& paramValue) { util::Params& p = *Rcpp::as>(params); + + // Check for zeros in the input---if we received these, the user is mistaken, + // because in R labels should start from 1. + if (arma::any(paramValue == 0)) + { + Log::Fatal << "When passing labels from R to mlpack, labels should be in " + << "the range from 1 to the number of classes!" << std::endl; + } + p.Get>(paramName) = paramValue - 1; p.SetPassed(paramName); } @@ -180,9 +196,9 @@ void SetParamUCol(SEXP params, // of a parameter. // [[Rcpp::export]] void SetParamMatWithInfo(SEXP params, - const std::string& paramName, - const LogicalVector& dimensions, - const arma::mat& paramValue) + const std::string& paramName, + const LogicalVector& dimensions, + const arma::mat& paramValue) { util::Params& p = *Rcpp::as>(params); data::DatasetInfo d(paramValue.n_cols); diff --git a/src/mlpack/bindings/R/mlpack/tests/testthat/test-R_binding.R b/src/mlpack/bindings/R/mlpack/tests/testthat/test-R_binding.R index ee77ad3c33..789f3056be 100644 --- a/src/mlpack/bindings/R/mlpack/tests/testthat/test-R_binding.R +++ b/src/mlpack/bindings/R/mlpack/tests/testthat/test-R_binding.R @@ -126,7 +126,7 @@ test_that("TestCol", { # Test an unsigned column vector input parameter. test_that("TestUCol", { - x <- matrix(as.integer(rexp(100, rate = .1)), nrow = 1) + x <- matrix(as.integer(rexp(100, rate = .1)), nrow = 1) + 1 output <- test_r_binding(4.0, 12, "hello", ucol_in=x) @@ -148,7 +148,7 @@ test_that("TestRow", { # Test an unsigned row vector input parameter. test_that("TestURow", { - x <- matrix(as.integer(rexp(100, rate = .1)), ncol = 1) + x <- matrix(as.integer(rexp(100, rate = .1)), ncol = 1) + 1 output <- test_r_binding(4.0, 12, "hello", urow_in=x) @@ -269,6 +269,15 @@ test_that("TestNotMatrix", { matrix_and_info_in=1e6)) }) +# If we pass labels that start from 0, we should get an error. +test_that("TestZeroLabels", { + x <- vector(mode="integer", 10) + expect_error(test_r_binding(4.0, 12, "hello", urow_in=x)) + + y <- matrix(0, 10, 1) + expect_error(test_r_binding(4.0, 12, "hello", ucol_in=y)) +}) + # First create a GaussianKernel object, then send it back and make sure we get # the right double value. test_that("TestModel", {