zdebruine / singlet

Single-cell analysis with non-negative matrix factorization
42 stars 13 forks source link

"Not a matrix." error when it's definitely a matrix #20

Closed Wainberg closed 1 year ago

Wainberg commented 1 year ago

All of these calls to cross_validate_nmf():

cross_validate_nmf(A=as(matrix(c(1,2,3,4), 2), 'dgCMatrix'), ranks=c(1,2), verbose=3)
cross_validate_nmf(A=as(matrix(c(1,2,3,4), 2), 'dgRMatrix'), ranks=c(1,2), verbose=3)
cross_validate_nmf(A=NormalizeData(get_pbmc3k_data())@assays$RNA@data, ranks=c(1,2), verbose=3)

give the same error:

running with sparse optimization
k = 1, rep = 1 (1/6):
Error in c_ard_nmf(A, At, tol, maxit, verbose > 1, L1, L2, threads, w_init[[rep]][1:df$k[[i]],  :
  Not a matrix.
Wainberg commented 1 year ago

Ah looks like the problem is that it doesn't support 1 NMF component. Changing ranks to c(2,3) works fine.

Wainberg commented 1 year ago

So maybe just do a bounds check for k if you're not going to support k = 1? Also worth checking for k <= 0: k = 0 currently gives the error Error in w_init[[rep]][1:df$k[[i]], ] : subscript out of bounds and k < 0 gives the error Error in stats::runif(nrow(A) * max(ranks)) : invalid arguments, neither of which really convey what the problem is.

zdebruine commented 1 year ago

Thanks, a few more checks are definitely needed here, your suggestions are great. We could support k = 1 on the C++ side no problem, it would be quite trivial actually as long as Rcpp gets a dgCMatrix or matrix format. That said, why not just run k = 1 SVD? Although k = 1 NMF can be faster with a dedicated implementation.

Wainberg commented 1 year ago

I think running k = 1 NMF is mostly to satisfy oneself that k = 2 is actually the optimum, in cases where k = 2 is picked during cross-validation. Sometimes there really is only one NMF component's worth of variation in your data.

zdebruine commented 1 year ago

Yes, or for establishing a one-dimensional linear diffusion gradient (i.e. pseudotime, trajectory, applications in minimum spanning trees, etc).

zdebruine commented 1 year ago

See this issue: https://github.com/zdebruine/singlet/issues/31.

Closing this due to duplicate.