#115 clean up try-catch irlba

jlmelville · May 18, 2024 · 60116b0 · 60116b0
1 parent 1fd789f
commit 60116b0
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 67 deletions.
diff --git a/R/init.R b/R/init.R
@@ -178,41 +178,42 @@ irlba_tsvd_normalized_laplacian_init <- function(A, ndim = 2, verbose = FALSE) {
 }
 
 irlba_spectral_tsvd <- function(L, n, iters = 1000) {
+  irlba_args <- list(
+    A = L,
+    nv = n,
+    nu = 0,
+    maxit = iters
+  )
   suppressWarnings(res <- tryCatch(
-    irlba::irlba(L, nv = n, nu = 0, maxit = iters),
+    do.call(irlba::irlba, irlba_args),
     error = function(c) {
-      irlba::irlba(L, nv = n, nu = 0, maxit = iters, fastpath = FALSE)
+      irlba_args$fastpath <- FALSE
+      do.call(irlba::irlba, irlba_args)
     }
   ))
-  list(vectors = res$v, values = 2.0 - res$d, converged = res$iter != iters)
+  list(
+    vectors = res$v,
+    values = 2.0 - res$d,
+    converged = res$iter != iters
+  )
 }
 
 irlba_eigs_asym <- function(L, ndim) {
+  irlba_args <- list(
+    x = L,
+    n = ndim + 1,
+    symmetric = FALSE,
+    smallest = TRUE,
+    tol = 1e-3,
+    maxit = 1000
+  )
   suppressWarnings(res <- tryCatch(
-    {
-      irlba::partial_eigen(
-        L,
-        n = ndim + 1,
-        symmetric = FALSE,
-        smallest = TRUE,
-        tol = 1e-3,
-        maxit = 1000,
-        verbose = TRUE
-      )
-    },
-    error = function(c) {
+    do.call(irlba::partial_eigen, irlba_args),
+    error = function(e) {
+      irlba_args$fastpath <- FALSE
       tryCatch(
-        irlba::partial_eigen(
-          L,
-          n = ndim + 1,
-          symmetric = FALSE,
-          smallest = TRUE,
-          tol = 1e-3,
-          maxit = 1000,
-          verbose = TRUE,
-          fastpath = FALSE
-        ),
-        error = function(c) {
+        do.call(irlba::partial_eigen, irlba_args),
+        error = function(e) {
           NULL
         }
       )
@@ -225,29 +226,21 @@ irlba_eigs_asym <- function(L, ndim) {
 }
 
 irlba_eigs_sym <- function(L, ndim, smallest = TRUE) {
+  irlba_args <- list(
+    x = L,
+    n = ndim + 1,
+    symmetric = TRUE,
+    smallest = smallest,
+    tol = 1e-3,
+    maxit = 1000
+  )
   suppressWarnings(res <- tryCatch(
-    irlba::partial_eigen(
-      L,
-      n = ndim + 1,
-      symmetric = TRUE,
-      smallest = smallest,
-      tol = 1e-3,
-      maxit = 1000,
-      verbose = FALSE
-    ),
-    error = function(c) {
+    do.call(irlba::partial_eigen, irlba_args),
+    error = function(e) {
+      irlba_args$fastpath <- FALSE
       tryCatch(
-        irlba::partial_eigen(
-          L,
-          n = ndim + 1,
-          symmetric = TRUE,
-          smallest = smallest,
-          tol = 1e-3,
-          maxit = 1000,
-          verbose = FALSE,
-          fastpath = FALSE
-        ),
-        error = function(c) {
+        do.call(irlba::partial_eigen, irlba_args),
+        error = function(e) {
           NULL
         }
       )

diff --git a/tests/testthat/test_normlaplacian.R b/tests/testthat/test_normlaplacian.R
@@ -32,21 +32,13 @@ test_that("normalized laplacian", {
 })
 
 test_that("irlba tsvd normalized", {
-  # 115: ensure irlba code path gets tested if we can avoid Matrix ABI issue
-  # if (exists("Matrix.Version", envir = asNamespace("Matrix")) &&
-  #     Matrix::Matrix.Version()$package >= "1.7.0") {
-    res <- irlba_tsvd_normalized_laplacian_init(sparse_m)
-    expect_equal(abs(res), abs_expected_norm_lap, tolerance = 0.2)
-  # }
+  res <- irlba_tsvd_normalized_laplacian_init(sparse_m)
+  expect_equal(abs(res), abs_expected_norm_lap, tolerance = 0.2)
 })
 
 test_that("irlba normalized", {
-  # 115: ensure irlba code path gets tested if we can avoid Matrix ABI issue
-  # if (exists("Matrix.Version", envir = asNamespace("Matrix")) &&
-  #     Matrix::Matrix.Version()$package >= "1.7.0") {
-    res <- irlba_normalized_laplacian_init(sparse_m)
-    expect_equal(abs(res), abs_expected_norm_lap, tolerance = 0.2)
-  # }
+  res <- irlba_normalized_laplacian_init(sparse_m)
+  expect_equal(abs(res), abs_expected_norm_lap, tolerance = 0.2)
 })
 
 test_that("laplacian eigenmap", {

diff --git a/tests/testthat/test_output.R b/tests/testthat/test_output.R
@@ -309,15 +309,11 @@ res <- umap(iris10,
 expect_ok_matrix(res, nc = 1)
 
 # enforce irlba for spectral initialization even if RSpectra is present
-# 115: ensure irlba code path gets tested if we can avoid Matrix ABI issue
-# if (exists("Matrix.Version", envir = asNamespace("Matrix")) &&
-#   Matrix::Matrix.Version()$package >= "1.7.0") {
-  res <- umap(iris10,
-    n_components = 1, n_neighbors = 4, n_epochs = 2,
-    n_threads = 1, verbose = FALSE, init = "irlba_spectral"
-  )
+res <- umap(iris10,
+  n_components = 1, n_neighbors = 4, n_epochs = 2,
+  n_threads = 1, verbose = FALSE, init = "irlba_spectral"
+)
   expect_ok_matrix(res, nc = 1)
-# }
 
 # Supervised
 set.seed(1337)