kuwisdelu / matter

Out-of-core statistical computing and signal processing
http://www.cardinalmsi.org
54 stars 4 forks source link

cbind not working when binding named matrices with different number of columns #9

Closed daniel-rodak closed 5 years ago

daniel-rodak commented 5 years ago

As in title. Here is reproducible example

library(matter)
vec1 <- sample(0:9, 1000, TRUE, prob = c(81, rep(1, 9)))
vec2 <- sample(0:9, 500, TRUE, prob = c(81, rep(1, 9)))
rows <- paste0("row", 1:50)
cols <- paste0("col", 1:30)
mat1 <- matrix(vec1, nrow = 50, ncol = 20, dimnames = list(rows, cols[1:20]))
mat2 <- matrix(vec2, nrow = 50, ncol = 10, dimnames = list(rows, cols[21:30]))

matt1 <- as.sparse(mat1)
matt2 <- as.sparse(mat2)
matt3 <- cbind(matt1, matt2)

Error message:

 Error in validObject(.Object) : 
  invalid class “sparse_matc” object: length of 'dimnames' [2] not equal to array extent 

I have also found the solution. Problem is in matter:::combine_colnames. Buggy part is like this:

    } else if ( is.null(dimnames(y)[[2]]) ) {
        colnames <- c(dimnames(x)[[2]], character(dim(x)[2]))
    } else {
        colnames <- c(dimnames(x)[[2]], dimnames(x)[[2]])

Should be like this. There is change from x to y in second arguments of c():

    } else if ( is.null(dimnames(y)[[2]]) ) {
        colnames <- c(dimnames(x)[[2]], character(dim(y)[2]))
    } else {
        colnames <- c(dimnames(x)[[2]], dimnames(y)[[2]])

Same problem in the similar part of code is also in matter:::combine_rownames.

Thanks, Daniel

kuwisdelu commented 5 years ago

Fixed on Github and added a test case. Thanks!