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

refactor div-conq SVD, impl svd_rand #165

Open
wants to merge 3 commits into
base: master
from

Conversation

@nlhepler
Copy link
Contributor

@nlhepler nlhepler commented Jun 27, 2019

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 to svd_dc (so we can have svd_rand), and moved svd_dc under the SVD_ 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 for Array2, but is needed for sparse representations of matrices (be they sprs or a low-rank representation like M = 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.

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"

This comment has been minimized.

@nlhepler

nlhepler Jun 28, 2019
Author Contributor

This is required for the Dot impl in @pmarks branch, which itself is dependent on rust-lang/rust#55437

This comment has been minimized.

@termoshtt

termoshtt Jul 27, 2019
Member

We need to create nightly CI

Copy link
Member

@termoshtt termoshtt left a comment

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"

This comment has been minimized.

@termoshtt

termoshtt Jul 27, 2019
Member

We need to create nightly CI

FlagSVD::None
}
}
}

This comment has been minimized.

@termoshtt

termoshtt Jul 27, 2019
Member

cool :)

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.

None yet

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