Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Put parameters values in sparsity / smoothness specification #48

Merged
merged 26 commits into from
Aug 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6255c88
Add `is_valid_parameters` and tests
Banana1530 Jul 20, 2019
b750bd3
Remove print in tests
Banana1530 Jul 20, 2019
c4f319d
Add extended argument helpers
Banana1530 Jul 20, 2019
83f3e96
Move to util.R
Banana1530 Jul 20, 2019
c5dc1c9
Absorb values into lasso(..)
Banana1530 Jul 21, 2019
7feddc7
Update test cases and comment out some of them
Banana1530 Jul 21, 2019
42a1862
Update pca wrappers with extended helper functions, and tests
Banana1530 Jul 21, 2019
ff43d48
Fix travis build
Banana1530 Jul 22, 2019
01b3410
More comments in test_sfpca_wrapper.R
Banana1530 Jul 25, 2019
d807332
Add comments in `create_moma_sparsity_func`
Banana1530 Aug 4, 2019
155819f
Add a warning for `mget(ls())`
Banana1530 Aug 5, 2019
0bb27a7
Modify comments in `check_omega`
Banana1530 Aug 5, 2019
3609d19
Use is_square
Banana1530 Aug 5, 2019
a73be3e
is_finite_numeric_scalar -> is_finite_numeric_scalar
Banana1530 Aug 5, 2019
5f7fa15
Update a comment
Banana1530 Aug 5, 2019
a035929
Add chkDots in SFPCA
Banana1530 Aug 9, 2019
0f9f1d5
"rank" -> "minimum-dimension"
Banana1530 Aug 9, 2019
16a5757
Add `error_if_not`
Banana1530 Aug 9, 2019
7f7877d
Update comments
Banana1530 Aug 9, 2019
c07b24e
Add a space
Banana1530 Aug 9, 2019
f36fc53
Add `error_if_not_valid_select_str`
Banana1530 Aug 9, 2019
752be4a
Change file name
Banana1530 Aug 9, 2019
594ceb8
Add `error_if_not_fourchar_bg_string`
Banana1530 Aug 9, 2019
30628f3
Update error message
Banana1530 Aug 9, 2019
81753c7
sapply -> vapply
Banana1530 Aug 9, 2019
031082b
Change defaults of special functions
Banana1530 Aug 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 64 additions & 5 deletions R/moma_arguments.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@
#' @name moma_sparsity
NULL

# Check whether `x` is a boolean value
is_logical_scalar <- function(x) {
return(is.logical(x) && (length(x) == 1) && !is.na(x))
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is moved to a new file util.R.

empty <- function() {
arglist <- list()
class(arglist) <- "moma_sparsity"
Expand Down Expand Up @@ -359,3 +354,67 @@ moma_pg_settings <- function(..., EPS = 1e-10, MAX_ITER = 1000,
class(arglist) <- "moma_pg_settings"
return(arglist)
}

create_moma_sparsity_func <- function(f) {
# Given f, we want to generate a new function, which
# 1. contains all arguments in f;
# 2. has two extra arguments `lambda` and `select_scheme`;
# 3. returns a list that contains ( f(...), lambda = ..., select_scheme = ... ).
aug_f <- function(..., lambda = 0, select_scheme = "g") {
chkDots(...)

# Step 2: check lambda
error_if_not_valid_parameters(lambda)

# Step 3: check select_scheme
error_if_not_valid_select_str(select_scheme)

# step 4: return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you comment a bit more on this design? The mget(ls()) construct seems very fragile and I'm not 100% sure what it's doing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added the following comments in the code. See commit Add comments in create_moma_sparsity_func.

   # Given f, we want to generate a new function, which
   # 1. contains all arguments in f;
   # 2. has two extra arguments `lambda` and `select_scheme`;
   # 3. returns a list that contains ( f(...), lambda = ..., select_scheme = ... ).

As for mget(ls()), I found it in a stackoverflow question and don't know how it works in detail. What I want is to get the list of arguments passed into the function (arg_for_f <- mget(ls())) and then exclude two augmented arguments lambda and select_scheme (arg_for_f <- arg_for_f[names(arg_for_f) %in% c("lambda", "select_scheme") == FALSE ] ) so that I can call the function f: (do.call(f, arg_for_f)).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ls() will give a character vector of all names bound in an environment and then mget will take that vector and return a list - taken together they are basically getting all the local variables and grouping them up.

That said, this will break if you introduce any new local variables in any of the code before this line.

Wouldn't it be easier to capture the dots:

dots <- list(...); sparsity_type <- do.call(f, dots)

or similar?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Emmm no, this won't work for current design. See explanations below.

# WARNING: do not define local variables before
# `mget(ls())`
arg_for_f <- mget(ls())
arg_for_f <- arg_for_f[
names(arg_for_f) %in% c("lambda", "select_scheme") == FALSE
] # fetch arguments for the function `f`
a <- list(
sparsity_type = do.call(f, arg_for_f),
lambda = lambda,
select_scheme = select_scheme
)
class(a) <- "moma_sparsity_type"
return(a)
}
formals(aug_f) <- c(formals(f), formals(aug_f))
Copy link
Collaborator Author

@Banana1530 Banana1530 Aug 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we augment the formals of aug_f. So by dots <- list(...) we won't be able to extract arguments required by the function f.

For example, lasso <- function(non_neg = FALSE) {...}. Then moma_lasso <- create_moma_sparsity_func(lasso) is almost equivalent to moma_lasso <- function(non_neg = FALSE, ..., lambda = 0, select_scheme = "g").

And the reason why I need formals(aug_f) <- c(formals(f), formals(aug_f)) is I want a drop-down list appears, when the user types moma_lasso( and presses tab for prompts for arguments.

In conclusion, formals(aug_f) <- c(formals(f), formals(aug_f)) enhances UX but requires we extract arguments for f, during the construction of aug_f in create_moma_sparsity_func , by means other than list(...).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I understand where you are going with the formals manipulation and it's quite nice: I'm still less comfortable with mget(ls()). Since there are only 10 of these and we don't have any plans to add more, is it worth it to move away from the wrapper function approach and just hand-write 10 small functions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I open an issue to track this #51.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can reduce our concerns here. The logic is clear and simple: We augment arguments of aug_f with those of f, so by the time we enter the ultimate aug_f, we are sure all local variables are those needed to call f, plus lambda and select_scheme.

aug_f
}

moma_empty <- create_moma_sparsity_func(empty)
moma_lasso <- create_moma_sparsity_func(lasso)
moma_mcp <- create_moma_sparsity_func(mcp)
moma_scad <- create_moma_sparsity_func(scad)
moma_slope <- create_moma_sparsity_func(slope)
moma_grplasso <- create_moma_sparsity_func(grplasso)
moma_fusedlasso <- create_moma_sparsity_func(fusedlasso)
moma_l1tf <- create_moma_sparsity_func(l1tf)
moma_spfusedlasso <- create_moma_sparsity_func(spfusedlasso)
moma_cluster <- create_moma_sparsity_func(cluster)

# What this function does now is just wrap three arguement
# into a list.
# TODO: `Omega` could be a user-defined function
moma_smoothness <- function(Omega = NULL, ..., alpha = 0, select_scheme = "g") {

# Step 2: check lambda
error_if_not_valid_parameters(alpha)

# Step 3: check select_scheme
error_if_not_valid_select_str(select_scheme)

a <- list(
Omega = Omega,
alpha = alpha,
select_scheme = select_scheme
)
class(a) <- "moma_smoothness_type"
return(a)
}
78 changes: 0 additions & 78 deletions R/moma_expose.R
Original file line number Diff line number Diff line change
@@ -1,81 +1,3 @@
MOMA_EMPTYMAT <- matrix()
MOMA_EMPTYVEC <- vector(mode = "numeric")
MOMA_DEFAULT_PROX <- list(
P = "NONE",
gamma = 3,
# non-negativity
nonneg = FALSE,
# grouping
group = MOMA_EMPTYVEC,
lambda2 = 0,
# unordered fusion
w = MOMA_EMPTYMAT,
ADMM = FALSE,
acc = FALSE,
prox_eps = 1e-10,
# trend filtering
l1tf_k = 1
)
add_default_prox_args <- function(sparsity_type) {
# sparsity_type: prox arguments for u and v

# To call a C function we have to specify
# all arguments. However, some arguments
# are specific for a particular prox. So
# we first assign a default arg list to
# `df_prox_arg_list_u/_v` and
# then update them.
return(modifyList(MOMA_DEFAULT_PROX, sparsity_type))
}

is.wholenumber <-
function(x, tol = .Machine$double.eps^0.5) abs(x - round(x)) < tol

# This function checks the validity of Omega and alpha
check_omega <- function(Omega, alpha, n) {

# check if Omega is a matrix
if (!is.matrix(Omega) && !is.null(Omega)) {
moma_error("Omega_u/v is not a matrix.")
}

# TODO: store them as sparse matrices using the package Matrix
if (length(alpha) == 1 && alpha == 0) {
# discard the Omega matrix specified by users
Omega <- diag(n)
}
else if (is.null(Omega)) {
# The user wants smooth penalty
# but does not specify Omega matrix
Omega <- second_diff_mat(n) # TODO: should not overwrite
}
else {
# At this point, users have specified an Omega and
# non-zero penalty levels explicitly

# Check validity of Omega if users speicify both alpha and Omega
if (dim(Omega)[1] != dim(Omega)[2]) {
moma_error(
"Omega shoud be a square matrix: nrows = ", dim(Omega)[1],
", ncols = ", dim(Omega)[2]
)
}
if (dim(Omega)[1] != n) {
moma_error(
"Omega shoud be a compatible matrix. It should be of ",
n, "x", n,
", but is actually ", dim(Omega)[1], "x", dim(Omega)[1]
)
}
# TODO: check definiteness and symmetry of Omega
}
return(Omega)
}

second_diff_mat <- function(n) {
return(crossprod(diff(diag(n))))
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is moved to a new file util.R.

moma_svd <- function(
X,
u_sparsity = empty(), v_sparsity = empty(), lambda_u = 0, lambda_v = 0, # lambda_u/_v is a vector or scalar
Expand Down
Loading