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

WIP sparse matrix classes #16

Merged
merged 2 commits into from May 30, 2019
Merged

WIP sparse matrix classes #16

merged 2 commits into from May 30, 2019

Conversation

@flying-sheep
Copy link
Contributor

@flying-sheep flying-sheep commented May 2, 2019

Fixes #15 when ready

svds_real_gen(A, k, nu, nv, opts, mattype = "dgRMatrix")
{
fun = if(isSymmetric(A)) svds_real_sym else svds_real_gen
fun(A, k, nu, nv, opts, mattype = "dgRMatrix")

This comment has been minimized.

@flying-sheep

flying-sheep May 2, 2019
Author Contributor

i just read above that “isSymmetric doesn’t support dgRMatrix”, but maybe we can convert dgRMatrix to dsRMatrix?

@@ -20,6 +20,10 @@
##' the \strong{Matrix} package.\cr
##' \code{dsyMatrix} \tab Symmetrix matrix, defined in the \strong{Matrix}

This comment has been minimized.

@flying-sheep

flying-sheep May 2, 2019
Author Contributor

I fixed that typo in the other file.

##' @rdname eigs
##' @export
## isSymmetric() does not support dgRMatrix
eigs.dgRMatrix <- function(A, k, which = "LM", sigma = NULL, opts = list(), ...)
eigs_real_gen(A, nrow(A), k, which, sigma, opts, mattype = "dgRMatrix")

##' @rdname eigs
##' @export
eigs.dsRMatrix <- eigs.dgRMatrix

This comment has been minimized.

@flying-sheep

flying-sheep May 2, 2019
Author Contributor

Without code delegating to eigs_real_sym in eigs.dgRMatrix, this is wrong.

)) {
if (which == "LR") which = "LA"
if (which == "SR") which = "SA"
eigs_real_sym(A, nrow(A), k, which, sigma, opts, mattype = "dsCMatrix",
extra_args = list(use_lower = TRUE))

This comment has been minimized.

@flying-sheep

flying-sheep May 2, 2019
Author Contributor

I think this should be split: extra_args should only be passed when calling this on a dgCMatrix, or we should forget about extra_args here and convert symmetric any dgCMatrix to a dsCMatrix.

@yixuan
Copy link
Owner

@yixuan yixuan commented May 3, 2019

This is great work. Thank you!
I'll take a look over the weekend.

@yixuan
Copy link
Owner

@yixuan yixuan commented May 9, 2019

@flying-sheep Thanks for your nice work! 👍
I have found some subtle issues in the current version, and you can start to fix them if you don't want to wait. Otherwise I will find a time to make edits based on yours.

@flying-sheep
Copy link
Contributor Author

@flying-sheep flying-sheep commented May 9, 2019

You can just push commits to my branch, no?

@yixuan
Copy link
Owner

@yixuan yixuan commented May 9, 2019

Oh, I can work on my side. I just meant that I might not have time until some weekends.

@yixuan yixuan merged commit 924bd52 into yixuan:master May 30, 2019
@yixuan
Copy link
Owner

@yixuan yixuan commented May 30, 2019

Just got some time to address these issues and I think it now fully supports dsCMatrix and dsRMatrix. Thank you!

@flying-sheep flying-sheep deleted the flying-sheep:sym branch May 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.