theislab / destiny

R package for single cell and other data analysis using diffusion maps
https://theislab.github.io/destiny/
GNU General Public License v3.0
77 stars 11 forks source link

Patch so no_censoring() doesn't crash through upper.tri() #4

Closed Loyale closed 6 years ago

Loyale commented 6 years ago

upper.tri() forcibly coerces input to a base matrix() using as.matrix(). This can be memory limiting for large sparse matrices. as.matrix() is not needed in upper.tri() to accomplish the goals of creating the mask in destiny. As such this patch defines upper.tri.sparse() in R/utils.r with exactly the same implementation as upper.tri() without the call to as.matrix(). This is replaced in R/diffusionmap.r and allows destiny to proceed on an otherwise large 10x genomics single cell dataset (>150K cells) on a 32 Gb machine. Thanks for destiny. Great package!

flying-sheep commented 6 years ago

Very cool, thank you! And I’m happy to hear you like destiny!

Would you mind formatting the new code like the package code? Then I’ll have no spurious commits only for formatting. The indentation is tabs, and i use whitespace around <-, =, and after ,.

Very elegant use of rows and cols 😄

Loyale commented 6 years ago

Should be formatted as requested now. Let me know if I can tweak anything else to match. Cheers.

flying-sheep commented 6 years ago

Thank you, it’s perfect!