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.

Sign up for GitHub

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

Jump to bottom

Clatrix not transposing V* matrix in correctly in SVD return #63

Closed
mikera opened this issue Apr 3, 2015 · 9 comments
Closed

Clatrix not transposing V* matrix in correctly in SVD return #63

mikera opened this issue Apr 3, 2015 · 9 comments

Comments

@mikera
Copy link
Collaborator

mikera commented Apr 3, 2015

core.matrix API specifics that V* should be the transpose of the V matrix in the SVD decomposition. Clatrix is currently returning the non-transposed V.

See mikera/vectorz-clj#47
@nblumoe
Copy link

nblumoe commented Jun 15, 2015

@mikera thanks for raising and fixing this over here.
Should we handle this somewhere upstream, too, until it gets fixed in clatrix?

I currently have pending changes to Incanter with fixed tests. However these tests fail now, when using the clatrix implementation. Of course I could add something to Incanter, but core.matrix would seem to be more appropriate.

More importantly: Can we get this fix merged, please?

@mikera mikera closed this as completed in 7ddf3ce Jun 15, 2015
@mikera
Copy link
Collaborator Author

mikera commented Jun 15, 2015

@nblumoe OK I merged it.... don't seem to have been any objections to this fix.

I can't make official Clatrix releases though

@tel
Copy link
Owner

tel commented Jun 15, 2015

Do you not have Clojars access? I can fix that.

—
Sent from Mailbox

On Mon, Jun 15, 2015 at 8:37 AM, Mike Anderson notifications@github.com
wrote:

@nblumoe OK I merged it.... don't seem to have been any objections to this fix.

I can't make official Clatrix releases though

Reply to this email directly or view it on GitHub:
#63 (comment)

@mikera
Copy link
Collaborator Author

mikera commented Jun 15, 2015

@tel - actually looks like I do have membership of the clatrix group on clojars....
a) does that mean I can make a release?
b) Should I? (this isn't my project after all!)

@tel
Copy link
Owner

tel commented Jun 15, 2015

Ah, great!

I haven’t been an active maintainer for quite some time, so I don’t want to weigh in on decisions like that. I just wanted to make sure there wasn’t any structural reason you couldn’t release :)

Pinging @Quantisan?

—
Sent from Mailbox

On Mon, Jun 15, 2015 at 10:41 AM, Mike Anderson notifications@github.com
wrote:

@tel - actually looks like I do have membership of the clatrix group on clojars....
a) does that mean I can make a release?

b) Should I? (this isn't my project after all!)

Reply to this email directly or view it on GitHub:
#63 (comment)

@Quantisan
Copy link
Collaborator

Please do @mikera
Looks to me like you've been the maintainer for this library for a while, so you make the call.

@tel
Copy link
Owner

tel commented Jun 15, 2015

I'm happy discussing anything necessary for the release as necessary as well.

@mikera
Copy link
Collaborator Author

mikera commented Jun 16, 2015

OK I created a 0.5.0 release - hope that works for everyone!

nblumoe added a commit to nblumoe/incanter that referenced this issue Jun 16, 2015
@nblumoe
Test for correctly transposed correlation matrix
e5b6926 
core.matrix expects V* to be the transposed matrix of the right singular
vectors. The corresponding test was fixed to reflect this.

Furthermore the clatrix version had to be bumped:
clatrix was returning the untransposed matrix V,
see tel/clatrix#63
@tel
Copy link
Owner

tel commented Jun 16, 2015

Great! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tel @mikera @Quantisan @nblumoe