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

Setting my_lambda and nlambda on MADMMplasso() #54

Closed
wleoncio opened this issue Aug 20, 2024 · 9 comments
Closed

Setting my_lambda and nlambda on MADMMplasso() #54

wleoncio opened this issue Aug 20, 2024 · 9 comments
Assignees
Labels
question Further information is requested

Comments

@wleoncio
Copy link
Member

wleoncio commented Aug 20, 2024

Quick question: when calling MADMMplasso(), does it make sense to set nlambda != ncol(y)? There doesn't seem to be anything in the code that prevents this, but this bit of code implies that if my_lambda = NULL, lambda_i is created with length ncol(y):

if (is.null(my_lambda)) {
r <- y
lammax <- lapply(
seq_len(ncol(y)),
function(g) {
max(abs(t(X) %*% (r - colMeans(r))) / length(r[, 1])) / ((1 - alpha) + (max(gg[1, ]) * max(CW) + max(gg[2, ])))
}
)
big_lambda <- lammax
lambda_i <- lapply(
seq_len(ncol(y)),
function(g) {
exp(seq(log(big_lambda[[g]]), log(big_lambda[[g]] * rat), length = maxgrid))
}
)

This then breaks this later bit of the code, since the vectors lam[, i] will have a different size than the vectors lambda_i[[i]]:

MADMMplasso/R/MADMMplasso.R

Lines 173 to 177 in 081383c

if (is.null(my_lambda)) {
lam <- matrix(0, nlambda, ncol(y))
for (i in seq_len(ncol(y))) {
lam[, i] <- lambda_i[[i]]
}

@wleoncio wleoncio added the question Further information is requested label Aug 20, 2024
@Theo-qua
Copy link
Collaborator

Hello Waldir,
nlambda cannot be equal to ncol(y). nlambda is for the number of rows of the lambda values and ncol(y) is the number of columns for lambda values (lam).

@wleoncio
Copy link
Member Author

OK, so that means one can't freely assign nlambda to whatever integer they want, right? Isn't nlambda determined by the number of rows on my_lambda (whether passed to MADMMplasso() or calculated inside it)?

@Theo-qua
Copy link
Collaborator

nlambda can be any positive integer so it can be freely specified. I was only saying that it not the same as ncol(y).

@wleoncio
Copy link
Member Author

OK, so why does this code give an error (main branch)?

@wleoncio
Copy link
Member Author

The code above gives an error of Error in gg[hh, ] : subscript out of bounds, implying a connection between gg and nlambda (through hh), but according to the documentation gg must be a 2x2 matrix, is that so?

@Theo-qua
Copy link
Collaborator

Yes, it gave error because of maxgrid = 1L, nlambda = 2L. According to the documentation, nlambda should be less than or equal to maxgrid.

@Theo-qua
Copy link
Collaborator

gg is regenerated in the function with a length equal to maxgrid before the loop.

@wleoncio
Copy link
Member Author

Great, thanks! I read that part, but didn't catch the exception. We should validate that nlambda <= maxgrid to avoid unclear error messages (and unnecessary computation), agreed? I can open a PR with a quick fix.

Regarding gg, I'm confused now. I can't find it being created inside MADMMplasso(), only used to calculate lammax, gg1, gg2 and gg3 (see here). gg even seems like a mandatory argument, as not passing it (or passing NULL, which is the default) results in error.

@Theo-qua
Copy link
Collaborator

I agree with you. I reassigned gg to gg3 in the MADMMplasso() after calculating gg3. I did not consider to cause any confusion since this is internal assignment. So gg is changed from the initial gg in the call to gg3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants