KlugerLab / FIt-SNE

Fast Fourier Transform-accelerated Interpolation-based t-SNE (FIt-SNE)
Other
593 stars 108 forks source link

need to require rsvd package in R wrapper #111

Closed mmp3 closed 3 years ago

mmp3 commented 3 years ago

The R wrapper is missing some require statements.

The following minimal reproducible example results in an error:

# FIt-SNE
source("/home/ubuntu/packages/FIt-SNE/fast_tsne.R" , chdir = TRUE)

mat  <-  matrix( data = rnorm(6e3*4e3) , nrow = 6e3 )
perplexity_list  <-  c(30, 200)
res  <-  fftRtsne( X = mat ,   learning_rate = 6e3/12 ,  initialization = "pca" ,  perplexity = 0 , perplexity_list = perplexity_list )

output:

Loading required namespace: rsvd
Using rsvd() to compute the top PCs for initialization.
Error in rsvd(X_c, k = dims) : could not find function "rsvd"

The error goes away after installing package rsvd from CRAN and then calling library('rsvd') before calling fftRtsne

linqiaozhi commented 3 years ago

Hi @mmp3, thanks for the issue report.

This error is introduce in a recent PR #103. We originally were using require() which gives a warning if the library is not installed. @DillonHammill suggested changing to requireNamespace(), because that fixes some R CMD CHECK warnings, but it functions differently: if the library does not exist, then it gives an error.

Dillon, do you have any thoughts? If require() was the wrong function, then what should we use here? Based on this thread, I was thinking to change it to:

"rsvd" %in% rownames(installed.packages())
DillonHammill commented 3 years ago

@linqiaozhi, there are a couple of options here:

  1. rsvd and irlba offer significant speed improvements to PCA computation compared to prcomp/princomp in the stats package. If prcomp/princomp produces the same result (I am not sure about this), you could check to see if either rsvd or irlba is installed and if not, you could resort to using prcomp/princomp from the stats package. All R users will have this package so the code will run without error every time.
  2. We could simply return FALSE if an error is encountered when looking for the rsvd or irlba packages so that the if/else statements proceed as expected. I have implemented this in PR #112 which now returns a more meaningful error message when either of these packages has not been installed. Doing things this way means that you can stick to using requireNamespace().
linqiaozhi commented 3 years ago

Fixed by @DillonHammill in #113. Thanks so much!