-
Notifications
You must be signed in to change notification settings - Fork 74
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
refactor div-conq SVD, impl svd_rand #165
base: master
Are you sure you want to change the base?
Conversation
credit to @pmarks for original svd_rand impl
Cargo.toml
Outdated
@@ -41,6 +41,10 @@ default-features = false | |||
version = "0.3" | |||
default-features = false | |||
|
|||
[dependencies.sprs] | |||
git = "https://github.com/pmarks/sprs.git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is required for the Dot impl in @pmarks branch, which itself is dependent on rust-lang/rust#55437
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to create nightly CI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
As you said, it will be better to split into dense and sparse part, and svddc refactoring part possibly.
This PR can be merged by omitting sparse part.
Before implementing sparse part, we need to create nightly CI for testing it, and CI setting will be my task.
Cargo.toml
Outdated
@@ -41,6 +41,10 @@ default-features = false | |||
version = "0.3" | |||
default-features = false | |||
|
|||
[dependencies.sprs] | |||
git = "https://github.com/pmarks/sprs.git" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to create nightly CI
FlagSVD::None | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool :)
credit to @pmarks for original svd_rand impl
This should probably be 2 PRs, but I wanted to post it here for discussion first.
First, I moved
svddc
tosvd_dc
(so we can havesvd_rand
), and movedsvd_dc
under theSVD_
trait, de-duplicating a lot of logic.I also implement
svd_rand
, though this version does not have the changes required for complex numbers, which we definitely want. This is dependent on a set of complex trait requirements that just works forArray2
, but is needed for sparse representations of matrices (be they sprs or a low-rank representation likeM = u.dot(&v)
.I also add an optional feature on sprs, though right now only @pmarks branch of sprs implements the Dot traits required to make
svd_rand
function.