search

mikera / vectorz-clj

Fast matrix and vector maths library for Clojure - as a core.matrix implementation
202 stars 19 forks source link

QR fails with rows < columns #32

Closed mikera closed 10 years ago

mikera commented 10 years ago 
(qr (matrix :vectorz [[1 2]]))
IllegalArgumentException Wrong matrix size: rows < columns  mikera.matrixx.decompose.QR.decomposeInternal (QR.java:66)

I think this should succeed with something like the following result: Q= [[1.0]] R= [[1.0 2.0]]

This seems to be a perfectly valid QR decomposition?

prasant94 commented 10 years ago

Yes, this issue won't arise with the Householder implementation. I think we should switch to that.

mikera commented 10 years ago

Well we should use the best implementation for each case. If Householder version is best for rows < cols then that may be the best option.

But we still want to use the fastest option available for rows == cols

prasant94 commented 10 years ago

I ran the two implementations for a whole bunch of different matrix sizes. I found that Householder implementation is faster as long as columns > 0.3*rows.

Here is the data:

for square matrices smaller than 100x100, householder is generally faster but sometimes the current one turns out to run faster. for square matrices b/w 100x100 to 1000x1000, householder is the clear winner. For rectangular matrices, householder is slower for really tall matrices but at some point around when the columns are 3/10th the number of rows, householder starts performing faster that the current implementation.

size current householder
100x100 67 65
200x200 185 23
300x300 156 48
400x400 542 113
500x500 1356 211
600x600 2749 361
700x700 4277 559
800x800 6121 873
900x900 10228 1483
1000x1000 14570 2345
1000x100 202 535
1000x200 538 730
1000x300 1064 937
1000x400 1982 1104
1000x500 3492 1262
600x60 138 185
600x120 120 96
600x180 199 135
600x240 347 176
600x300 575 202
600x360 855 245
mikera commented 10 years ago

Interesting. Seems that householder version is doing a lot better in most cases?

Any idea why the current version is much slower? Is there a performance issue that needs to be fixed?

Otherwise it looks like we should switch to the householder version.

mikera commented 10 years ago

Is this fixed now? @prasant94 feel free to close these issues once they are done....

prasant94 commented 10 years ago

yes, this is fixed.