From b7f502aeaa6886b8e923bf9e7b6fdfb2325e1b50 Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Mon, 24 Jun 2019 20:09:20 +0800
Subject: [PATCH 1/9] Add R wrapper for PG loop settings

---
 R/moma_arguments.R | 13 +++++++++++++
 R/moma_svd.R       | 22 ++++++++++++----------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/R/moma_arguments.R b/R/moma_arguments.R
index 38e33279..b86309e0 100644
--- a/R/moma_arguments.R
+++ b/R/moma_arguments.R
@@ -275,3 +275,16 @@ cluster <- function(w = NULL, ADMM = FALSE,
     class(arglist) <- "moma_sparsity"
     return(arglist)
 }
+
+moma_pg_setting <- function(EPS = 1e-10, MAX_ITER = 1000,
+                            EPS_inner = 1e-10, MAX_ITER_inner = 1e+5,
+                            solver = c("ista", "fista", "onestepista"), ...) {
+    solver <- match.arg(solver)
+    arglist <- list(
+        EPS = EPS, MAX_ITER = MAX_ITER,
+        EPS_inner = EPS_inner, MAX_ITER_inner = MAX_ITER_inner,
+        solver = toupper(solver)
+    )
+    class(arglist) <- "moma_pg_setting"
+    return(arglist)
+}
diff --git a/R/moma_svd.R b/R/moma_svd.R
index c86460bd..1b932bf0 100644
--- a/R/moma_svd.R
+++ b/R/moma_svd.R
@@ -67,9 +67,7 @@ moma_svd <- function(
                      X,
                      u_sparsity = empty(), v_sparsity = empty(), lambda_u = 0, lambda_v = 0, # lambda_u/_v is a vector or scalar
                      Omega_u = NULL, Omega_v = NULL, alpha_u = 0, alpha_v = 0, # so is alpha_u/_v
-                     EPS = 1e-10, MAX_ITER = 1000,
-                     EPS_inner = 1e-10, MAX_ITER_inner = 1e+5,
-                     solver = "ista",
+                     pg_setting = moma_pg_setting(),
                      k = 1, # number of pairs of singular vecters
                      select = c("gridsearch", "nestedBIC")) {
     if (!inherits(alpha_u, c("numeric", "integer")) ||
@@ -116,12 +114,6 @@ moma_svd <- function(
         # smoothness
         alpha_u = alpha_u,
         alpha_v = alpha_v,
-        # algorithm parameters
-        EPS = EPS,
-        MAX_ITER = MAX_ITER,
-        EPS_inner = EPS_inner,
-        MAX_ITER_inner = MAX_ITER_inner,
-        solver = toupper(solver),
         k = k
     )
 
@@ -157,6 +149,15 @@ moma_svd <- function(
         )
     }
 
+    # PG loop settings
+    if (!inherits(pg_setting, "moma_pg_setting")) {
+        moma_error(
+            "pg_setting penalty should be of class ",
+            sQuote("moma_pg_setting"),
+            ". Try using, for example, `pg_setting = moma_pg_setting(MAX_ITER=1e+4)`."
+        )
+    }
+
     # Pack all argument into a list
     # First we check the smoothness term argument.
     algo_settings_list <- c(
@@ -166,7 +167,8 @@ moma_svd <- function(
             Omega_v = check_omega(Omega_v, alpha_v, p),
             prox_arg_list_u = add_default_prox_args(u_sparsity),
             prox_arg_list_v = add_default_prox_args(v_sparsity)
-        )
+        ),
+        pg_setting
     )
 
     if (is_multiple_para) {

From 2fce4b96132b596e9076b8ac95b04aa32e17d5a7 Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Mon, 24 Jun 2019 20:10:36 +0800
Subject: [PATCH 2/9] Add tests for the wrapper

---
 src/moma.cpp                    |  4 ++-
 tests/testthat/test_arguments.R | 43 +++++++++++++++++++++++++--------
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/src/moma.cpp b/src/moma.cpp
index beae5c78..5d706786 100644
--- a/src/moma.cpp
+++ b/src/moma.cpp
@@ -60,7 +60,9 @@ MoMA::MoMA(const arma::mat &i_X,  // Pass X_ as a reference to avoid copy
     MoMALogger::info("Initializing MoMA object:")
         << " lambda_u " << lambda_u << " lambda_v " << lambda_v << " alpha_u " << alpha_u
         << " alpha_v " << alpha_v << " P_u " << Rcpp::as<std::string>(i_prox_arg_list_u["P"])
-        << " P_v " << Rcpp::as<std::string>(i_prox_arg_list_v["P"]);
+        << " P_v " << Rcpp::as<std::string>(i_prox_arg_list_v["P"]) << " EPS " << i_EPS
+        << " MAX_ITER " << i_MAX_ITER << " EPS_inner " << i_EPS_inner << " MAX_ITER_inner "
+        << i_MAX_ITER_inner << " solver " << i_solver;
     // Step 2: Initialize to leading singular vectors
     //
     //         MoMA is a regularized SVD, which is a non-convex (bi-convex)
diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index 9b190be2..e1abbd00 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -26,11 +26,16 @@ test_that("Test for arguments names", {
     args <- names(cluster(w = matrix(1)))
     test_args <- c(test_args, args)
 
-    # Test
+    # Test prox argumetns
     for (arg in test_args) {
         expect_true(paste0(arg, "_u") %in% correct_args)
         expect_true(paste0(arg, "_v") %in% correct_args)
     }
+
+    # Test PG loop arguments
+    for (arg in names(moma_pg_setting())) {
+        expect_true(arg %in% correct_args)
+    }
 })
 
 test_that("Prompt errors for wrong prox arguments", {
@@ -298,9 +303,23 @@ test_that("Correct prox match", {
         ),
         "Initializing a fusion lasso proximal operator object"
     )
+
+    expect_output(
+        moma_svd(matrix(runif(12), 3, 4),
+            u_sparsity = cluster(diag(3), ADMM = TRUE), lambda_u = 3
+        ),
+        "Running ADMM"
+    )
+
+    expect_output(
+        moma_svd(matrix(runif(12), 3, 4),
+            u_sparsity = cluster(diag(3)), lambda_u = 3
+        ),
+        "Running AMA"
+    )
 })
 
-test_that("Correct algorithm match", {
+test_that("Correct match for PG loop settings", {
     old_logger_level <- MoMA::moma_logger_level()
     MoMA::moma_logger_level("DEBUG")
 
@@ -332,19 +351,23 @@ test_that("Correct algorithm match", {
         "Releasing a OneStepISTA object"
     )
 
-
+    # Test default PG loop setting
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = cluster(diag(3), ADMM = TRUE), lambda_u = 3
-        ),
-        "Running ADMM"
+        moma_svd(matrix(runif(12), 3, 4)),
+        "EPS 1e-10 MAX_ITER 1000 EPS_inner 1e-10 MAX_ITER_inner 100000 solver ISTA"
     )
 
+    # Test pg_setting() passes correct values to C++ side
     expect_output(
         moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = cluster(diag(3)), lambda_u = 3
-        ),
-        "Running AMA"
+            pg_setting = moma_pg_setting(
+                EPS = 1.212312e-5,
+                MAX_ITER = 1.2957e+7,
+                EPS_inner = 1.987e-6,
+                MAX_ITER_inner = 98728376
+            )
+        ),
+        "EPS 1.21231e-05 MAX_ITER 12957000 EPS_inner 1.987e-06 MAX_ITER_inner 98728376"
     )
 
     on.exit(MoMA::moma_logger_level(old_logger_level))

From 5ae091597cd9e4b6fbf51e27d279ad20652b5406 Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Mon, 24 Jun 2019 20:10:49 +0800
Subject: [PATCH 3/9] Update old tests

---
 tests/testthat/test_arguments.R | 12 ++++++------
 tests/testthat/test_grid.R      |  8 ++++----
 tests/testthat/test_sfpca.R     |  6 +++---
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index e1abbd00..ad5eec6a 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -325,29 +325,29 @@ test_that("Correct match for PG loop settings", {
 
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), solver = "ista"),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "ista")),
         "Initializing a ISTA solver"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), solver = "fista"),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "fista")),
         "Initializing a FISTA solver"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), solver = "onestepista"),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "onestepista")),
         "Initializing an one-step ISTA solver"
     )
 
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), solver = "ista"),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "ista")),
         "Releasing a ISTA object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), solver = "fista"),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "fista")),
         "Releasing a FISTA object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), solver = "onestepista"),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "onestepista")),
         "Releasing a OneStepISTA object"
     )
 
diff --git a/tests/testthat/test_grid.R b/tests/testthat/test_grid.R
index 85b143d6..3d87e236 100644
--- a/tests/testthat/test_grid.R
+++ b/tests/testthat/test_grid.R
@@ -26,12 +26,12 @@ test_that("Using cpp_sfpca_grid is equivalent to run cpp_sfpca multiple times",
         ista.cv <- moma_svd(X,
             Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm_set,
             lambda_u = 0, lambda_v = sp_set, u_sparsity = lasso(), v_sparsity = sptype(),
-            EPS = 1e-14, MAX_ITER = 1e+5, solve = "ISTA", EPS_inner = 1e-9
+            pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "ista", EPS_inner = 1e-9)
         )
         fista.cv <- moma_svd(X,
             Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm_set,
             lambda_u = 0, lambda_v = sp_set, u_sparsity = lasso(), v_sparsity = sptype(),
-            EPS = 1e-14, MAX_ITER = 1e+5, solve = "FISTA", EPS_inner = 1e-9
+            pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "fista", EPS_inner = 1e-9)
         )
         cnt <- 1
         for (sp in sp_set) {
@@ -39,12 +39,12 @@ test_that("Using cpp_sfpca_grid is equivalent to run cpp_sfpca multiple times",
                 ista <- moma_svd(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm,
                     lambda_u = 0, lambda_v = sp, u_sparsity = lasso(), v_sparsity = sptype(),
-                    EPS = 1e-14, MAX_ITER = 1e+5, solve = "ISTA", EPS_inner = 1e-9
+                    pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "ista", EPS_inner = 1e-9)
                 )
                 fista <- moma_svd(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm,
                     lambda_u = 0, lambda_v = sp, u_sparsity = lasso(), v_sparsity = sptype(),
-                    EPS = 1e-14, MAX_ITER = 1e+5, solve = "FISTA", EPS_inner = 1e-9
+                    pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "fista", EPS_inner = 1e-9)
                 )
 
                 # Cannot use expect_equal here due to numerical error
diff --git a/tests/testthat/test_sfpca.R b/tests/testthat/test_sfpca.R
index b7b4d840..106036ef 100644
--- a/tests/testthat/test_sfpca.R
+++ b/tests/testthat/test_sfpca.R
@@ -71,7 +71,7 @@ test_that("Closed-form solution when no sparsity imposed", {
                 # WARNING: One-step ISTA does not pass this test
                 res <- sfpca(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = a_u, alpha_v = a_v,
-                    EPS = 1e-7, MAX_ITER = 1e+5, solve = solver
+                    EPS = 1e-7, MAX_ITER = 1e+5, solver = solver
                 )
 
                 # The sfpca solutions and the svd solutions are related by an `L` matrix
@@ -112,12 +112,12 @@ test_that("ISTA and FISTA should yield similar results,
                 ista <- sfpca(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = sp, alpha_v = sp,
                     lambda_u = sm, lambda_v = sm, P_u = "LASSO", P_v = sptype,
-                    EPS = 1e-14, MAX_ITER = 1e+3, solve = "ISTA", EPS_inner = 1e-9
+                    EPS = 1e-14, MAX_ITER = 1e+3, solver = "ISTA", EPS_inner = 1e-9
                 )
                 fista <- sfpca(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = sp, alpha_v = sp,
                     lambda_u = sm, lambda_v = sm, P_u = "LASSO", P_v = sptype,
-                    EPS = 1e-6, MAX_ITER = 1e+3, solve = "FISTA", EPS_inner = 1e-9
+                    EPS = 1e-6, MAX_ITER = 1e+3, solver = "FISTA", EPS_inner = 1e-9
                 )
 
                 # WARNING: We observe if zero appears in either v or u, ista and fista

From 5c531d4027724e05c592587d382796f54ebf9214 Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Thu, 27 Jun 2019 15:26:59 +0800
Subject: [PATCH 4/9] fix typo

---
 tests/testthat/test_arguments.R | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index ad5eec6a..bec2d8b0 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -26,7 +26,7 @@ test_that("Test for arguments names", {
     args <- names(cluster(w = matrix(1)))
     test_args <- c(test_args, args)
 
-    # Test prox argumetns
+    # Test prox arguments
     for (arg in test_args) {
         expect_true(paste0(arg, "_u") %in% correct_args)
         expect_true(paste0(arg, "_v") %in% correct_args)

From 9501a6739da6085fe6ae56d44eb354f29f15358c Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Thu, 27 Jun 2019 18:53:51 +0800
Subject: [PATCH 5/9] Add ellipses, update old tests, and add new tests

---
 R/moma_arguments.R              |  59 ++++-
 tests/testthat/test_arguments.R | 409 ++++++++++++++++++++++----------
 2 files changed, 336 insertions(+), 132 deletions(-)

diff --git a/R/moma_arguments.R b/R/moma_arguments.R
index b86309e0..67c0e50b 100644
--- a/R/moma_arguments.R
+++ b/R/moma_arguments.R
@@ -15,6 +15,7 @@ empty <- function() {
 #' \deqn{\lambda \sum \| x_{i} \| ,}
 #' where \eqn{\lambda} is set by \code{lambda_u/v} in the function \code{moma_svd}.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param non_negative a boolean value. Set \code{TRUE} to add non-negativity
 #' constraint.
 #'
@@ -23,7 +24,11 @@ empty <- function() {
 #' @examples
 #' lasso(non_negative = FALSE)
 #' @export
-lasso <- function(non_negative = FALSE) {
+lasso <- function(..., non_negative = FALSE) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
+
     if (!is_logical_scalar(non_negative)) {
         moma_error(sQuote("non_negative"), " should be a boolean value.")
     }
@@ -40,6 +45,7 @@ lasso <- function(non_negative = FALSE) {
 #' determined by \eqn{\gamma}. See Zhang, Cun-Hui. "Nearly unbiased variable
 #' selection under minimax concave penalty." The Annals of statistics 38.2 (2010): 894-942.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param gamma non-convexity. Must be larger than 1.
 #' @param non_negative a boolean value. Set to \code{TRUE} to add non-negativity
 #' constraint.
@@ -50,7 +56,11 @@ lasso <- function(non_negative = FALSE) {
 #' @examples
 #' mcp(gamma = 3, non_negative = FALSE)
 #' @export
-mcp <- function(gamma = 3, non_negative = FALSE) {
+mcp <- function(..., gamma = 3, non_negative = FALSE) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
+
     if (!is_logical_scalar(non_negative)) {
         moma_error(sQuote("non_negative"), " should be a boolean value.")
     }
@@ -75,6 +85,7 @@ mcp <- function(gamma = 3, non_negative = FALSE) {
 #'  via nonconcave penalized likelihood and its oracle properties." Journal of
 #'  the American statistical Association 96.456 (2001): 1348-1360.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param gamma non-convexity. Must be larger than 2.
 #' @param non_negative a boolean value. Set to \code{TRUE} to add non-negativity
 #' constraint.
@@ -85,7 +96,11 @@ mcp <- function(gamma = 3, non_negative = FALSE) {
 #' @examples
 #' scad(gamma = 3.7, non_negative = FALSE)
 #' @export
-scad <- function(gamma = 3.7, non_negative = FALSE) {
+scad <- function(..., gamma = 3.7, non_negative = FALSE) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
+
     if (!is_logical_scalar(non_negative)) {
         moma_error(sQuote("non_negative"), " should be a boolean value.")
     }
@@ -128,6 +143,7 @@ slope <- function() {
 #' where \eqn{\lambda} is set by \code{lambda_u/v} in the function \code{moma_svd}, \eqn{\|x_g\|} is
 #' the vector comprised of elements of \eqn{x} picked out by indeces set \eqn{g}.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param g a vector of integer or characters, or a factor itself. It gets transformed
 #' to factor eventually to indicate grouping.
 #' @param non_negative a boolean value. Set to \code{TRUE} to add non-negativity
@@ -140,7 +156,11 @@ slope <- function() {
 #' # This sets every three adjacent parameters as a group.
 #' grplasso(g = rep(1:10, each = 3), non_negative = FALSE)
 #' @export
-grplasso <- function(g, non_negative = FALSE) {
+grplasso <- function(..., g, non_negative = FALSE) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
+
     if (!is_logical_scalar(non_negative)) {
         moma_error(sQuote("non_negative"), " should be a boolean value.")
     }
@@ -157,6 +177,7 @@ grplasso <- function(g, non_negative = FALSE) {
 #' Use this function to set the penalty function as fused lasso
 #' \deqn{\lambda \sum \| x_{i} - x_{i-1} \|,}
 #' where \eqn{\lambda} is set by \code{lambda_u/v} in the function \code{moma_svd}.
+#' @param ... Forces users to specify all arguments by name.
 #' @param algo a string being either "path" or "dp". Defaults to "path". Partial matching
 #' is supported. Two solving algorithms
 #' are provided. When "path" is chosen, the algorithm by
@@ -173,7 +194,10 @@ grplasso <- function(g, non_negative = FALSE) {
 #' @examples
 #' fusedlasso()
 #' @export
-fusedlasso <- function(algo = c("path", "dp")) {
+fusedlasso <- function(..., algo = c("path", "dp")) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
     # fused lasso
 
     # Two options for solving the proximal operator
@@ -203,6 +227,7 @@ fusedlasso <- function(algo = c("path", "dp")) {
 #' Tibshirani, Ryan J. "Adaptive piecewise polynomial estimation via trend
 #' filtering." The Annals of Statistics 42.1 (2014): 285-323.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param l1tf_k use (k+1)-difference matrix in trend filtering. Note \eqn{k = 0}
 #'          implies piecewise constant, \eqn{k=1} implies piecewise linear, \eqn{k=2}
 #'          piecewise quadratic etc.
@@ -212,7 +237,10 @@ fusedlasso <- function(algo = c("path", "dp")) {
 #' @examples
 #' l1tf(l1tf_k = 1)
 #' @export
-l1tf <- function(l1tf_k = 1) {
+l1tf <- function(..., l1tf_k = 1) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
     # l1 linear trend filtering
     arglist <- list(P = "L1TRENDFILTERING", l1tf_k = l1tf_k)
     class(arglist) <- "moma_sparsity"
@@ -226,6 +254,7 @@ l1tf <- function(l1tf_k = 1) {
 #' where \eqn{\lambda_} is set by \code{lambda_u/v} in the function \code{moma_svd}, and \eqn{\lambda_2}
 #' is specified in here.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param lambda2 the level of penalty on the absolute values of the coefficients
 #'
 #' @return a \code{moma_sparsity} object, which is a list containing the value of \code{lambda_2}.
@@ -233,7 +262,10 @@ l1tf <- function(l1tf_k = 1) {
 #' @examples
 #' spfusedlasso(lambda2 = 2)
 #' @export
-spfusedlasso <- function(lambda2) {
+spfusedlasso <- function(..., lambda2) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
     arglist <- list(P = "SPARSEFUSEDLASSO", lambda2 = lambda2)
     class(arglist) <- "moma_sparsity"
     return(arglist)
@@ -245,6 +277,7 @@ spfusedlasso <- function(lambda2) {
 #' \deqn{\lambda \sum w_{ij} \| x_{i} - x_{j} \|,}
 #' where \eqn{\lambda} is set by \code{lambda_u/v} in the function \code{moma_svd}.
 #'
+#' @param ... Forces users to specify all arguments by name.
 #' @param w a symmetric square matrix. \code{w[i][j]} is the \eqn{w_{ij}} described above.
 #' @param ADMM a boolean value. Set to \code{TRUE} to use ADMM, set to \code{FALSE} to use AMA.
 #' @param acc a boolean value. Set to \code{TRUE} to use the accelereated version of the algorithm.
@@ -257,9 +290,12 @@ spfusedlasso <- function(lambda2) {
 #' @examples
 #' cluster(w = matrix(rep(1, 9), 3), ADMM = FALSE, acc = FALSE, eps = 1e-10)
 #' @export
-cluster <- function(w = NULL, ADMM = FALSE,
+cluster <- function(..., w = NULL, ADMM = FALSE,
                     acc = FALSE,
                     eps = 1e-10) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
     # fused lasso
     if (!is.matrix(w) || is.null(w) || dim(w)[1] != dim(w)[2]) {
         moma_error("`w` should be a square matrix.")
@@ -276,9 +312,12 @@ cluster <- function(w = NULL, ADMM = FALSE,
     return(arglist)
 }
 
-moma_pg_setting <- function(EPS = 1e-10, MAX_ITER = 1000,
+moma_pg_setting <- function(..., EPS = 1e-10, MAX_ITER = 1000,
                             EPS_inner = 1e-10, MAX_ITER_inner = 1e+5,
-                            solver = c("ista", "fista", "onestepista"), ...) {
+                            solver = c("ista", "fista", "onestepista")) {
+    if (length(list(...)) != 0) {
+        moma_error("Please specify the correct argument by name.")
+    }
     solver <- match.arg(solver)
     arglist <- list(
         EPS = EPS, MAX_ITER = MAX_ITER,
diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index bec2d8b0..c0cdd4d4 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -45,69 +45,103 @@ test_that("Prompt errors for wrong prox arguments", {
 
 
     # Wrong non-convexity arguments
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        u_sparsity = scad(1), lambda_u = 3
-    ),
-    paste0("Non-convexity parameter of SCAD (", sQuote("gamma"), ") must be larger than 2."),
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = scad(gamma = 1),
+            lambda_u = 3
+        ),
+        paste0(
+            "Non-convexity parameter of SCAD (",
+            sQuote("gamma"),
+            ") must be larger than 2."
+        ),
+        fixed = TRUE
     )
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        u_sparsity = mcp(0.9), lambda_u = 3
-    ),
-    paste0("Non-convexity parameter of MCP (", sQuote("gamma"), ") must be larger than 1."),
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = mcp(gamma = 0.9),
+            lambda_u = 3
+        ),
+        paste0(
+            "Non-convexity parameter of MCP (",
+            sQuote("gamma"),
+            ") must be larger than 1."
+        ),
+        fixed = TRUE
     )
 
 
     # Wrong grouping dimension in group lasso
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        u_sparsity = grplasso(factor(1)), lambda_u = 3
-    ),
-    "Wrong dimension: length(group) != dim(x).",
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = grplasso(g = factor(1)),
+            lambda_u = 3
+        ),
+        "Wrong dimension: length(group) != dim(x).",
+        fixed = TRUE
     )
-    expect_error(grplasso(matrix(1)),
+    expect_error(
+        grplasso(g = matrix(1)),
         "Please provide a vector as an indicator of grouping. (Called from grplasso)",
         fixed = TRUE
     )
 
 
     # Wrong weight matrix dimension in cluster penalty
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        u_sparsity = cluster(matrix(1)), lambda_u = 3
-    ),
-    "Wrong dimension: dim(weight matrix) != dim(x).",
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = cluster(w = matrix(1)),
+            lambda_u = 3
+        ),
+        "Wrong dimension: dim(weight matrix) != dim(x).",
+        fixed = TRUE
     )
 
 
     # Omega has wrong dimension
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        Omega_u = matrix(c(1, 2), 1, 2), alpha_u = 2
-    ),
-    "Omega shoud be a square matrix: nrows = 1, ncols = 2 (Called from check_omega)",
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            Omega_u = matrix(c(1, 2), 1, 2),
+            alpha_u = 2
+        ),
+        "Omega shoud be a square matrix: nrows = 1, ncols = 2 (Called from check_omega)",
+        fixed = TRUE
     )
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        Omega_u = matrix(c(1), 1), alpha_u = 2
-    ),
-    "Omega shoud be a compatible matrix. It should be of 3x3, but is actually 1x1 (Called from check_omega)",
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            Omega_u = matrix(c(1), 1),
+            alpha_u = 2
+        ),
+        "Omega shoud be a compatible matrix. It should be of 3x3, but is actually 1x1 (Called from check_omega)",
+        fixed = TRUE
     )
 
 
     # Prompt errors when users require rank-k svd and cross validation
-    expect_error(moma_svd(matrix(runif(12), 3, 4), lambda_u = c(1, 2, 3), k = 2),
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            lambda_u = c(1, 2, 3),
+            k = 2
+        ),
         "We don't support a range of parameters in finding a rank-k svd (Called from moma_svd)",
         fixed = TRUE
     )
-    expect_error(moma_svd(matrix(runif(12), 3, 4),
-        lambda_u = c(1, 2, 3),
-        lambda_v = seq(10),
-        alpha_u = seq(10)
-    ),
-    "We only allow changing two parameters.",
-    fixed = TRUE
+    expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            lambda_u = c(1, 2, 3),
+            lambda_v = seq(10),
+            alpha_u = seq(10)
+        ),
+        "We only allow changing two parameters.",
+        fixed = TRUE
     )
 })
 
@@ -123,28 +157,36 @@ test_that("Correct prox match", {
     )
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = lasso(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = lasso(),
+            lambda_u = 3
         ),
         "Initializing Lasso proximal operator object"
     )
 
     # SLOPE
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = slope(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = slope(),
+            lambda_u = 3
         ),
         "P_u SLOPE"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = slope(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = slope(),
+            lambda_u = 3
         ),
         "Initializing SLOPE proximal operator object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            v_sparsity = slope(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            v_sparsity = slope(),
+            lambda_u = 3
         ),
         "P_v SLOPE"
     )
@@ -157,14 +199,18 @@ test_that("Correct prox match", {
 
     # lasso
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = lasso(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = lasso(),
+            lambda_u = 3
         ),
         "Initializing Lasso proximal operator object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = lasso(TRUE), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = lasso(non_negative = TRUE),
+            lambda_u = 3
         ),
         "Initializing non-negative Lasso proximal operator object"
     )
@@ -172,14 +218,18 @@ test_that("Correct prox match", {
 
     # scad
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = scad(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = scad(),
+            lambda_u = 3
         ),
         "Initializing SCAD proximal operator object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = scad(non_negative = TRUE), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = scad(non_negative = TRUE),
+            lambda_u = 3
         ),
         "Initializing non-negative SCAD proximal operator object"
     )
@@ -187,14 +237,18 @@ test_that("Correct prox match", {
 
     # mcp
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = mcp(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = mcp(),
+            lambda_u = 3
         ),
         "Initializing MCP proximal operator object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = mcp(non_negative = TRUE), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = mcp(non_negative = TRUE),
+            lambda_u = 3
         ),
         "Initializing non-negative MCP proximal operator object"
     )
@@ -202,14 +256,18 @@ test_that("Correct prox match", {
 
     # group
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = grplasso(factor(seq(3))), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = grplasso(g = factor(seq(3))),
+            lambda_u = 3
         ),
         "Initializing group lasso proximal operator object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = grplasso(factor(seq(3)), non_negative = TRUE), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = grplasso(g = factor(seq(3)), non_negative = TRUE),
+            lambda_u = 3
         ),
         "Initializing non-negative group lasso proximal operator object"
     )
@@ -217,59 +275,76 @@ test_that("Correct prox match", {
 
     # L1 linear trend filtering
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = l1tf(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = l1tf(),
+            lambda_u = 3
         ),
         "Initializing a L1 linear trend filtering proximal operator object of degree 1"
     )
     expect_output(
-        moma_svd(matrix(runif(100), 10, 10),
-            u_sparsity = l1tf(l1tf_k = 2), lambda_u = 3
+        moma_svd(
+            matrix(runif(100), 10, 10),
+            u_sparsity = l1tf(l1tf_k = 2),
+            lambda_u = 3
         ),
         "Initializing a L1 linear trend filtering proximal operator object of degree 2"
     )
     expect_error(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = l1tf(l1tf_k = 2), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = l1tf(l1tf_k = 2),
+            lambda_u = 3
         ),
         "A difference matrix should have more columns."
     )
 
     # sparse fused lasso
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = spfusedlasso(lambda2 = 3), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = spfusedlasso(lambda2 = 3),
+            lambda_u = 3
         ),
         "Initializing a sparse fused lasso proximal operator object"
     )
     # fused lasso
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = fusedlasso(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = fusedlasso(),
+            lambda_u = 3
         ),
         "Initializing a ordered fusion lasso proximal operator object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = fusedlasso(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = fusedlasso(),
+            lambda_u = 3
         ),
         "P_u ORDEREDFUSED P_v NONE"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            v_sparsity = fusedlasso(), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            v_sparsity = fusedlasso(),
+            lambda_u = 3
         ),
         "P_u NONE P_v ORDEREDFUSED"
     )
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = fusedlasso(algo = "dp"), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = fusedlasso(algo = "dp"),
+            lambda_u = 3
         ),
         "P_u ORDEREDFUSEDDP P_v NONE"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
+        moma_svd(
+            matrix(runif(12), 3, 4),
             u_sparsity = fusedlasso(algo = "dp"),
             v_sparsity = fusedlasso(),
             lambda_u = 3
@@ -277,43 +352,55 @@ test_that("Correct prox match", {
         "P_u ORDEREDFUSEDDP P_v ORDEREDFUSED"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = fusedlasso(algo = "dp"), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = fusedlasso(algo = "dp"),
+            lambda_u = 3
         ),
         "P_u ORDEREDFUSEDDP P_v NONE"
     )
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = fusedlasso(algo = "dp"), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = fusedlasso(algo = "dp"),
+            lambda_u = 3
         ),
         "Initializing a ordered fusion lasso proximal operator object \\(DP\\)"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            v_sparsity = fusedlasso(algo = "dp"), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            v_sparsity = fusedlasso(algo = "dp"),
+            lambda_u = 3
         ),
         "Initializing a ordered fusion lasso proximal operator object \\(DP\\)"
     )
 
     # cluster penalty
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = cluster(diag(3)), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = cluster(w = diag(3)),
+            lambda_u = 3
         ),
         "Initializing a fusion lasso proximal operator object"
     )
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = cluster(diag(3), ADMM = TRUE), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = cluster(w = diag(3), ADMM = TRUE),
+            lambda_u = 3
         ),
         "Running ADMM"
     )
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
-            u_sparsity = cluster(diag(3)), lambda_u = 3
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            u_sparsity = cluster(w = diag(3)),
+            lambda_u = 3
         ),
         "Running AMA"
     )
@@ -359,7 +446,8 @@ test_that("Correct match for PG loop settings", {
 
     # Test pg_setting() passes correct values to C++ side
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4),
+        moma_svd(
+            matrix(runif(12), 3, 4),
             pg_setting = moma_pg_setting(
                 EPS = 1.212312e-5,
                 MAX_ITER = 1.2957e+7,
@@ -409,63 +497,140 @@ test_that("Negative penalty", {
     # Negative penalty
     set.seed(112)
     X <- matrix(runif(12), 3, 4)
-    expect_error(moma_svd(X = X, lambda_u = c(0, 1, 2, 3, 4, -1)),
+    expect_error(
+        moma_svd(X = X, lambda_u = c(0, 1, 2, 3, 4, -1)),
         paste0(
             "All penalty levels (",
-            sQuote("lambda_u"), ", ",
-            sQuote("lambda_v"), ", ",
-            sQuote("alpha_u"), ", ",
-            sQuote("alpha_v"), ") must be non-negative numeric. "
+            sQuote("lambda_u"),
+            ", ",
+            sQuote("lambda_v"),
+            ", ",
+            sQuote("alpha_u"),
+            ", ",
+            sQuote("alpha_v"),
+            ") must be non-negative numeric. "
         ),
         fixed = TRUE
     )
 
 
-    expect_error(moma_svd(X = X, lambda_v = c(0, 1, 2, 3, 4, -1)),
+    expect_error(
+        moma_svd(X = X, lambda_v = c(0, 1, 2, 3, 4, -1)),
         paste0(
             "All penalty levels (",
-            sQuote("lambda_u"), ", ",
-            sQuote("lambda_v"), ", ",
-            sQuote("alpha_u"), ", ",
-            sQuote("alpha_v"), ") must be non-negative numeric. "
+            sQuote("lambda_u"),
+            ", ",
+            sQuote("lambda_v"),
+            ", ",
+            sQuote("alpha_u"),
+            ", ",
+            sQuote("alpha_v"),
+            ") must be non-negative numeric. "
         ),
         fixed = TRUE
     )
 
     # Prompt error when penalty contains Infty
-    expect_error(moma_svd(X = X, lambda_v = c(1:3, Inf)),
+    expect_error(
+        moma_svd(X = X, lambda_v = c(1:3, Inf)),
         paste0(
             "All penalty levels (",
-            sQuote("lambda_u"), ", ",
-            sQuote("lambda_v"), ", ",
-            sQuote("alpha_u"), ", ",
-            sQuote("alpha_v"), ") must be non-negative numeric."
+            sQuote("lambda_u"),
+            ", ",
+            sQuote("lambda_v"),
+            ", ",
+            sQuote("alpha_u"),
+            ", ",
+            sQuote("alpha_v"),
+            ") must be non-negative numeric."
         ),
         fixed = TRUE
     )
 
     # Prompt error when passing a matrix
-    expect_error(moma_svd(X = X, lambda_v = matrix(1:12, 3)),
+    expect_error(
+        moma_svd(X = X, lambda_v = matrix(1:12, 3)),
         paste0(
             "All penalty levels (",
-            sQuote("lambda_u"), ", ",
-            sQuote("lambda_v"), ", ",
-            sQuote("alpha_u"), ", ",
-            sQuote("alpha_v"), ") must be numeric."
+            sQuote("lambda_u"),
+            ", ",
+            sQuote("lambda_v"),
+            ", ",
+            sQuote("alpha_u"),
+            ", ",
+            sQuote("alpha_v"),
+            ") must be numeric."
         ),
         fixed = TRUE
     )
 
+    expect_no_error(moma_svd(
+        X = X,
+        lambda_v = 1,
+        lambda_u = 1
+    ))
+})
 
 
-    expect_no_error(moma_svd(X = X, lambda_v = 1, lambda_u = 1),
-        paste0(
-            "All penalty levels (",
-            sQuote("lambda_u"), ", ",
-            sQuote("lambda_v"), ", ",
-            sQuote("alpha_u"), ", ",
-            sQuote("alpha_v"), ") must be non-negative numeric."
-        ),
-        fixed = TRUE
+test_that("Arguments must be specified by name", {
+    # lasso
+    expect_error(
+        lasso(FALSE),
+        "Please specify the correct argument by name"
+    )
+
+    expect_no_error(lasso(non_negative = TRUE))
+
+    # MCP
+    expect_error(
+        mcp(3),
+        "Please specify the correct argument by name"
+    )
+
+    expect_error(
+        mcp(3, non_negative = FALSE),
+        "Please specify the correct argument by name"
+    )
+    expect_error(
+        mcp(gamma = 3, TRUE),
+        "Please specify the correct argument by name"
+    )
+
+
+    # grplasso
+    expect_error(
+        grplasso(factor(1)),
+        "Please specify the correct argument by name"
+    )
+    expect_no_error(grplasso(g = factor(1)))
+
+
+    # fused lasso
+    expect_error(
+        fusedlasso("path"),
+        "Please specify the correct argument by name"
+    )
+    expect_no_error(fusedlasso(algo = "path"))
+
+    # trend filtering
+    expect_error(l1tf(1), "Please specify the correct argument by name")
+    expect_no_error(l1tf(l1tf_k = 1))
+
+    # sparse fused lasso
+    expect_error(
+        spfusedlasso(4),
+        "Please specify the correct argument by name"
+    )
+    expect_no_error(spfusedlasso(lambda2 = 4))
+
+    # clustering
+    expect_error(cluster(diag(3)), "Please specify the correct argument by name")
+    expect_no_error(cluster(w = diag(3)))
+
+    # PG loop settings
+    expect_error(
+        moma_pg_setting(1e-10),
+        "Please specify the correct argument by name"
     )
+    expect_no_error(moma_pg_setting(EPS = 1e-10))
 })

From b83a87fa053786d55b53bc7558f1a9e7a4f51ff1 Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Thu, 27 Jun 2019 18:56:03 +0800
Subject: [PATCH 6/9] moma_pg_setting -> moma_pg_settings

---
 R/moma_arguments.R              |  4 ++--
 R/moma_svd.R                    |  8 ++++----
 tests/testthat/test_arguments.R | 20 ++++++++++----------
 tests/testthat/test_grid.R      |  8 ++++----
 4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/R/moma_arguments.R b/R/moma_arguments.R
index 67c0e50b..b11d869c 100644
--- a/R/moma_arguments.R
+++ b/R/moma_arguments.R
@@ -312,7 +312,7 @@ cluster <- function(..., w = NULL, ADMM = FALSE,
     return(arglist)
 }
 
-moma_pg_setting <- function(..., EPS = 1e-10, MAX_ITER = 1000,
+moma_pg_settings <- function(..., EPS = 1e-10, MAX_ITER = 1000,
                             EPS_inner = 1e-10, MAX_ITER_inner = 1e+5,
                             solver = c("ista", "fista", "onestepista")) {
     if (length(list(...)) != 0) {
@@ -324,6 +324,6 @@ moma_pg_setting <- function(..., EPS = 1e-10, MAX_ITER = 1000,
         EPS_inner = EPS_inner, MAX_ITER_inner = MAX_ITER_inner,
         solver = toupper(solver)
     )
-    class(arglist) <- "moma_pg_setting"
+    class(arglist) <- "moma_pg_settings"
     return(arglist)
 }
diff --git a/R/moma_svd.R b/R/moma_svd.R
index 1b932bf0..df88d841 100644
--- a/R/moma_svd.R
+++ b/R/moma_svd.R
@@ -67,7 +67,7 @@ moma_svd <- function(
                      X,
                      u_sparsity = empty(), v_sparsity = empty(), lambda_u = 0, lambda_v = 0, # lambda_u/_v is a vector or scalar
                      Omega_u = NULL, Omega_v = NULL, alpha_u = 0, alpha_v = 0, # so is alpha_u/_v
-                     pg_setting = moma_pg_setting(),
+                     pg_setting = moma_pg_settings(),
                      k = 1, # number of pairs of singular vecters
                      select = c("gridsearch", "nestedBIC")) {
     if (!inherits(alpha_u, c("numeric", "integer")) ||
@@ -150,11 +150,11 @@ moma_svd <- function(
     }
 
     # PG loop settings
-    if (!inherits(pg_setting, "moma_pg_setting")) {
+    if (!inherits(pg_setting, "moma_pg_settings")) {
         moma_error(
             "pg_setting penalty should be of class ",
-            sQuote("moma_pg_setting"),
-            ". Try using, for example, `pg_setting = moma_pg_setting(MAX_ITER=1e+4)`."
+            sQuote("moma_pg_settings"),
+            ". Try using, for example, `pg_setting = moma_pg_settings(MAX_ITER=1e+4)`."
         )
     }
 
diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index c0cdd4d4..15a76de5 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -33,7 +33,7 @@ test_that("Test for arguments names", {
     }
 
     # Test PG loop arguments
-    for (arg in names(moma_pg_setting())) {
+    for (arg in names(moma_pg_settings())) {
         expect_true(arg %in% correct_args)
     }
 })
@@ -412,29 +412,29 @@ test_that("Correct match for PG loop settings", {
 
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "ista")),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_settings(solver = "ista")),
         "Initializing a ISTA solver"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "fista")),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_settings(solver = "fista")),
         "Initializing a FISTA solver"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "onestepista")),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_settings(solver = "onestepista")),
         "Initializing an one-step ISTA solver"
     )
 
 
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "ista")),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_settings(solver = "ista")),
         "Releasing a ISTA object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "fista")),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_settings(solver = "fista")),
         "Releasing a FISTA object"
     )
     expect_output(
-        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_setting(solver = "onestepista")),
+        moma_svd(matrix(runif(12), 3, 4), pg_setting = moma_pg_settings(solver = "onestepista")),
         "Releasing a OneStepISTA object"
     )
 
@@ -448,7 +448,7 @@ test_that("Correct match for PG loop settings", {
     expect_output(
         moma_svd(
             matrix(runif(12), 3, 4),
-            pg_setting = moma_pg_setting(
+            pg_setting = moma_pg_settings(
                 EPS = 1.212312e-5,
                 MAX_ITER = 1.2957e+7,
                 EPS_inner = 1.987e-6,
@@ -629,8 +629,8 @@ test_that("Arguments must be specified by name", {
 
     # PG loop settings
     expect_error(
-        moma_pg_setting(1e-10),
+        moma_pg_settings(1e-10),
         "Please specify the correct argument by name"
     )
-    expect_no_error(moma_pg_setting(EPS = 1e-10))
+    expect_no_error(moma_pg_settings(EPS = 1e-10))
 })
diff --git a/tests/testthat/test_grid.R b/tests/testthat/test_grid.R
index 3d87e236..51c34bd6 100644
--- a/tests/testthat/test_grid.R
+++ b/tests/testthat/test_grid.R
@@ -26,12 +26,12 @@ test_that("Using cpp_sfpca_grid is equivalent to run cpp_sfpca multiple times",
         ista.cv <- moma_svd(X,
             Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm_set,
             lambda_u = 0, lambda_v = sp_set, u_sparsity = lasso(), v_sparsity = sptype(),
-            pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "ista", EPS_inner = 1e-9)
+            pg_setting = moma_pg_settings(EPS = 1e-14, MAX_ITER = 1e+5, solver = "ista", EPS_inner = 1e-9)
         )
         fista.cv <- moma_svd(X,
             Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm_set,
             lambda_u = 0, lambda_v = sp_set, u_sparsity = lasso(), v_sparsity = sptype(),
-            pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "fista", EPS_inner = 1e-9)
+            pg_setting = moma_pg_settings(EPS = 1e-14, MAX_ITER = 1e+5, solver = "fista", EPS_inner = 1e-9)
         )
         cnt <- 1
         for (sp in sp_set) {
@@ -39,12 +39,12 @@ test_that("Using cpp_sfpca_grid is equivalent to run cpp_sfpca multiple times",
                 ista <- moma_svd(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm,
                     lambda_u = 0, lambda_v = sp, u_sparsity = lasso(), v_sparsity = sptype(),
-                    pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "ista", EPS_inner = 1e-9)
+                    pg_setting = moma_pg_settings(EPS = 1e-14, MAX_ITER = 1e+5, solver = "ista", EPS_inner = 1e-9)
                 )
                 fista <- moma_svd(X,
                     Omega_u = O_u, Omega_v = O_v, alpha_u = 0, alpha_v = sm,
                     lambda_u = 0, lambda_v = sp, u_sparsity = lasso(), v_sparsity = sptype(),
-                    pg_setting = moma_pg_setting(EPS = 1e-14, MAX_ITER = 1e+5, solver = "fista", EPS_inner = 1e-9)
+                    pg_setting = moma_pg_settings(EPS = 1e-14, MAX_ITER = 1e+5, solver = "fista", EPS_inner = 1e-9)
                 )
 
                 # Cannot use expect_equal here due to numerical error

From 9fba7d6ec9e81d9802094c8ad9df9d9285cfc7ee Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Thu, 27 Jun 2019 16:49:19 +0800
Subject: [PATCH 7/9] Add a test

---
 tests/testthat/test_arguments.R | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index 15a76de5..41dd4d57 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -458,6 +458,19 @@ test_that("Correct match for PG loop settings", {
         "EPS 1.21231e-05 MAX_ITER 12957000 EPS_inner 1.987e-06 MAX_ITER_inner 98728376"
     )
 
+     expect_error(
+        moma_svd(
+            matrix(runif(12), 3, 4),
+            pg_setting = c(
+                EPS = 1.212312e-5,
+                MAX_ITER = 1.2957e+7,
+                EPS_inner = 1.987e-6,
+                MAX_ITER_inner = 98728376
+            )
+        ),
+        paste0("pg_setting penalty should be of class ", sQuote("moma_pg_settings"))
+    )
+
     on.exit(MoMA::moma_logger_level(old_logger_level))
 })
 

From a22cdcf194de4b06c4866e60bdd0638516c8bde5 Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Thu, 27 Jun 2019 18:34:00 +0800
Subject: [PATCH 8/9] Spacing

---
 R/moma_arguments.R              | 4 ++--
 tests/testthat/test_arguments.R | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/R/moma_arguments.R b/R/moma_arguments.R
index b11d869c..40981a3c 100644
--- a/R/moma_arguments.R
+++ b/R/moma_arguments.R
@@ -313,8 +313,8 @@ cluster <- function(..., w = NULL, ADMM = FALSE,
 }
 
 moma_pg_settings <- function(..., EPS = 1e-10, MAX_ITER = 1000,
-                            EPS_inner = 1e-10, MAX_ITER_inner = 1e+5,
-                            solver = c("ista", "fista", "onestepista")) {
+                             EPS_inner = 1e-10, MAX_ITER_inner = 1e+5,
+                             solver = c("ista", "fista", "onestepista")) {
     if (length(list(...)) != 0) {
         moma_error("Please specify the correct argument by name.")
     }
diff --git a/tests/testthat/test_arguments.R b/tests/testthat/test_arguments.R
index 41dd4d57..648da426 100644
--- a/tests/testthat/test_arguments.R
+++ b/tests/testthat/test_arguments.R
@@ -458,7 +458,7 @@ test_that("Correct match for PG loop settings", {
         "EPS 1.21231e-05 MAX_ITER 12957000 EPS_inner 1.987e-06 MAX_ITER_inner 98728376"
     )
 
-     expect_error(
+    expect_error(
         moma_svd(
             matrix(runif(12), 3, 4),
             pg_setting = c(

From 3f81cf8bb0ad6a773e0506adc9c3cf400aaea1eb Mon Sep 17 00:00:00 2001
From: Luofeng Liao <luofengl@student.unimelb.edu.au>
Date: Tue, 25 Jun 2019 18:42:22 +0800
Subject: [PATCH 9/9] Variable naming: arg_list->public_arg_list

---
 tests/testthat/test_BIC_gird_mixed.R | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/testthat/test_BIC_gird_mixed.R b/tests/testthat/test_BIC_gird_mixed.R
index 9a271260..39dd059a 100644
--- a/tests/testthat/test_BIC_gird_mixed.R
+++ b/tests/testthat/test_BIC_gird_mixed.R
@@ -14,7 +14,7 @@ lambda_u <- seq(0.3, 1, length.out = n_lambda_u)
 lambda_v <- seq(0.3, 1, length.out = n_lambda_v)
 
 
-arg_list <- list(
+public_arg_list <- list(
     X,
     alpha_u = alpha_u, alpha_v = alpha_v,
     Omega_u = second_diff_mat(3), Omega_v = second_diff_mat(4),
@@ -28,7 +28,7 @@ test_that("BIC search returns correct-sized grid: four grid requests", {
     result <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 0, # grid
                 selection_criterion_alpha_v = 0, # grid
@@ -58,7 +58,7 @@ test_that("BIC search returns correct-sized grid: three grid requests", {
     result2 <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 0, # grid
                 selection_criterion_alpha_v = 0, # grid
@@ -83,7 +83,7 @@ test_that("BIC search returns correct-sized grid: three grid requests", {
     result2 <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 1,
                 selection_criterion_alpha_v = 0, # grid
@@ -110,7 +110,7 @@ test_that("BIC search returns correct-sized grid: two grid requests on u", {
     result3 <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 0, # grid
                 selection_criterion_lambda_u = 0, # grid
@@ -137,7 +137,7 @@ test_that("BIC search returns correct-sized grid: two grid requests on different
     result4 <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 1,
                 selection_criterion_lambda_u = 0, # grid
@@ -164,7 +164,7 @@ test_that("BIC search returns correct-sized grid: one grid", {
     result4 <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 1,
                 selection_criterion_lambda_u = 1,
@@ -191,7 +191,7 @@ test_that("BIC search returns correct-sized grid: all BIC search", {
     result4 <- do.call(
         testnestedBIC,
         c(
-            arg_list,
+            public_arg_list,
             list(
                 selection_criterion_alpha_u = 1,
                 selection_criterion_lambda_u = 1,
@@ -214,7 +214,7 @@ test_that("BIC search returns correct-sized grid: all BIC search", {
 
 test_that("testnestedBIC receivs a vector of length 0", {
     arglist <- c(
-        arg_list,
+        public_arg_list,
         list(
             selection_criterion_alpha_u = 1,
             selection_criterion_lambda_u = 1,