Skip to content

Commit

Permalink
Temporarily use OLD_extract_sparse_array for unknown sparse parsing.
Browse files Browse the repository at this point in the history 
The new extract_sparse_array() isn't well supported right now. Once it
is, we'll switch back and remove support for the legacy SparseArraySeed.
  • Loading branch information
@LTLA
LTLA committed Feb 14, 2024
1 parent fab7861 commit 6ddb9ab
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 14 deletions.
25 changes: 16 additions & 9 deletions inst/include/tatami_r/COO_SparseMatrix.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,28 @@
namespace tatami_r {

template<typename Data_ = double, typename Index_ = int, class InputObject_>
Parsed<Data_, Index_> parse_COO_SparseMatrix_internal(Rcpp::RObject seed, InputObject_ val, bool prefer_csr) {
Parsed<Data_, Index_> parse_COO_SparseMatrix_internal(Rcpp::RObject seed, InputObject_ val, bool prefer_csr, bool legacy) {
auto dims = parse_dims(seed.slot("dim"));
int NR = dims.first;
int NC = dims.second;

Rcpp::IntegerMatrix temp_i(Rcpp::RObject(seed.slot("nzcoo")));
std::string index_name;
if (legacy) {
index_name = "nzindex";
} else {
index_name = "nzcoo";
}

Rcpp::IntegerMatrix temp_i(Rcpp::RObject(seed.slot(index_name)));
if (temp_i.ncol() != 2) {
auto ctype = get_class_name(seed);
throw std::runtime_error(std::string("'nzcoo' slot in a ") + ctype + " object should have two columns");
throw std::runtime_error(std::string("'" + index_name + "' slot in a ") + ctype + " object should have two columns");
}

const size_t nnz = temp_i.nrow();
if (nnz != static_cast<size_t>(val.size())) {
auto ctype = get_class_name(seed);
throw std::runtime_error(std::string("incompatible 'nzcoo' and 'nzdata' lengths in a ") + ctype + " object");
throw std::runtime_error(std::string("incompatible '" + index_name + "' and 'nzdata' lengths in a ") + ctype + " object");
}

auto row_indices = temp_i.column(0);
Expand All @@ -38,7 +45,7 @@ Parsed<Data_, Index_> parse_COO_SparseMatrix_internal(Rcpp::RObject seed, InputO
auto check_index = [&](int r, int c) -> void {
if (r <= 0 || r > NR || c <= 0 || c > NC) {
auto ctype = get_class_name(seed);
throw std::runtime_error(std::string("'nzcoo' out of bounds in a ") + ctype + " object");
throw std::runtime_error(std::string("'" + index_name + "' out of bounds in a ") + ctype + " object");
}
};

Expand Down Expand Up @@ -182,16 +189,16 @@ Parsed<Data_, Index_> parse_COO_SparseMatrix_internal(Rcpp::RObject seed, InputO
}

template<typename Data_ = double, typename Index_ = int>
Parsed<Data_, Index_> parse_COO_SparseMatrix(Rcpp::RObject seed, bool prefer_csr) {
Parsed<Data_, Index_> parse_COO_SparseMatrix(Rcpp::RObject seed, bool prefer_csr, bool legacy=false) {
Rcpp::RObject vals(seed.slot("nzdata"));

Parsed<Data_, Index_> output;
if (vals.sexp_type() == REALSXP) {
output = parse_COO_SparseMatrix_internal<Data_, Index_>(seed, Rcpp::NumericVector(vals), prefer_csr);
output = parse_COO_SparseMatrix_internal<Data_, Index_>(seed, Rcpp::NumericVector(vals), prefer_csr, legacy);
} else if (vals.sexp_type() == INTSXP) {
output = parse_COO_SparseMatrix_internal<Data_, Index_>(seed, Rcpp::IntegerVector(vals), prefer_csr);
output = parse_COO_SparseMatrix_internal<Data_, Index_>(seed, Rcpp::IntegerVector(vals), prefer_csr, legacy);
} else if (vals.sexp_type() == LGLSXP) {
output = parse_COO_SparseMatrix_internal<Data_, Index_>(seed, Rcpp::LogicalVector(vals), prefer_csr);
output = parse_COO_SparseMatrix_internal<Data_, Index_>(seed, Rcpp::LogicalVector(vals), prefer_csr, legacy);
} else {
auto ctype = get_class_name(seed);
throw std::runtime_error("unsupported SEXP type (" + std::to_string(vals.sexp_type()) + ") for a " + ctype + "object");
Expand Down
11 changes: 6 additions & 5 deletions inst/include/tatami_r/UnknownMatrix.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ class UnknownMatrix : public tatami::Matrix<Value_, Index_> {
UnknownMatrix(Rcpp::RObject seed, size_t cache = -1) :
original_seed(seed),
delayed_env(Rcpp::Environment::namespace_env("DelayedArray")),
sparse_env(Rcpp::Environment::namespace_env("SparseArray")),
dense_extractor(delayed_env["extract_array"]),
sparse_extractor(sparse_env["extract_sparse_array"])
sparse_extractor(delayed_env["OLD_extract_sparse_array"])
{
// We assume the constructor only occurs on the main thread, so we
// won't bother locking things up. I'm also not sure that the
Expand Down Expand Up @@ -117,7 +116,7 @@ class UnknownMatrix : public tatami::Matrix<Value_, Index_> {
Index_ chunk_nrow, chunk_ncol;

Rcpp::RObject original_seed;
Rcpp::Environment delayed_env, sparse_env;
Rcpp::Environment delayed_env;
Rcpp::Function dense_extractor, sparse_extractor;

public:
Expand Down Expand Up @@ -323,7 +322,7 @@ class UnknownMatrix : public tatami::Matrix<Value_, Index_> {
if (parsed_primary != work->primary_block_len || parsed_secondary != work->secondary_len) {
auto ctype = get_class_name(original_seed);
throw std::runtime_error("'" +
(sparse_err ? std::string("extract_sparse_array") : std::string("extract_array")) +
(sparse_err ? std::string("OLD_extract_sparse_array") : std::string("extract_array")) +
"(<" + ctype + ">)' returns incorrect dimensions");
}
}
Expand Down Expand Up @@ -404,8 +403,10 @@ class UnknownMatrix : public tatami::Matrix<Value_, Index_> {
parsed = parse_SVT_SparseMatrix<Value_, Index_>(val0);
} else if (ctype == "COO_SparseMatrix") {
parsed = parse_COO_SparseMatrix<Value_, Index_>(val0, byrow_);
} else if (ctype == "SparseArraySeed") {
parsed = parse_COO_SparseMatrix<Value_, Index_>(val0, byrow_, /* legacy */ true);
} else {
throw std::runtime_error("unknown class '" + ctype + "' returned from 'extract_sparse_array()'");
throw std::runtime_error("unknown class '" + ctype + "' returned from 'OLD_extract_sparse_array()'");
}
check_buffered_dims<byrow_, true, true>(parsed.matrix.get(), work);

Expand Down
7 changes: 7 additions & 0 deletions tests/testthat/test-initializeCpp-other.R
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,11 @@ test_that("initialization works correctly with an unknown DelayedArray", {

ptr <- initializeCpp(mat2)
am_i_ok(mat, ptr)

# works in the sparse case.
mat <- DelayedArray(Matrix::rsparsematrix(100, 50, 0.1))
mat2 <- round(mat, digits=2)

expect_warning(ptr <- initializeCpp(mat2), "falling back")
am_i_ok(mat2, ptr)
})

0 comments on commit 6ddb9ab

Please sign in to comment.