bnprks / BPCells

Scaling Single Cell Analysis to Millions of Cells
https://bnprks.github.io/BPCells
Other
166 stars 17 forks source link

Latest version break the `as.matrix` operation #97

Closed Yunuuuu closed 4 months ago

Yunuuuu commented 4 months ago
mock_matrix <- function(ngenes, ncells) {
    cell.means <- 2^stats::runif(ngenes, 2, 10)
    cell.disp <- 100 / cell.means + 0.5
    cell.data <- matrix(stats::rnbinom(ngenes * ncells,
        mu = cell.means,
        size = 1 / cell.disp
    ), ncol = ncells)
    rownames(cell.data) <- sprintf("Gene_%s", formatC(seq_len(ngenes),
        width = 4, flag = 0
    ))
    colnames(cell.data) <- sprintf("Cell_%s", formatC(seq_len(ncells),
        width = 3, flag = 0
    ))
    cell.data
}
path <- tempfile()
mat <- mock_matrix(200, 40)
obj <- BPCells::write_matrix_dir(mat = as(mat, "dgCMatrix"), dir = path)
#> Warning: Matrix compression performs poorly with non-integers.
#> • Consider calling convert_matrix_type if a compressed integer matrix is intended.
#> This message is displayed once every 8 hours.
obj <- obj[1:100L, 1:20]
mat <- mat[1:100L, 1:20]
values <- matrix(sample(mat, length(mat)), nrow = nrow(mat))
obj <- BPCells::convert_matrix_type(obj, "uint32_t") # if we omit this line, it'll work fine
obj[1:10, ] <- values[1:10, ]
#> Warning: Converting input matrix type to match destination
#> • input type: double
#> • destination type: uint32_t
as.matrix(obj)
#> Warning: Converting to a dense matrix may use excessive memory
#> This message is displayed once every 8 hours.
#> Error in h(simpleError(msg, call)): error in evaluating the argument 'x' in selecting a method for function 'as.matrix': invalid class "dgTMatrix" object: length of Dimnames[[1]] (10) is not equal to Dim[1] (100)

Created on 2024-07-03 with reprex v2.1.0 ~
~
~
~
~
~
~
~
~

bnprks commented 4 months ago

Hi @Yunuuuu, thanks for your clear report! This seems to be yet another bug with our dimnames strategy which will hopefully be refactored to be a bit less precarious to write in the future.

This particular error and a related one I found based on it should be fixed, at least.

-Ben