lauerfab / MLweb

Machine learning and scientific computing (linear algebra, statistics, optimization) javascript libraries, with an online lab.
http://mlweb.loria.fr
GNU General Public License v2.0
85 stars 22 forks source link

Infinite loop in svd function #5

Open mooreryan opened 6 years ago

mooreryan commented 6 years ago

For some matrices, this loop starts to loop infinitely:

https://github.com/lauerfab/MLweb/blob/bf9a67c03372dcf21aade28984806d2486e4a665/lalolab/src/linalg.js#L6938

I did some debugging, and this if statement appears to always be true:

https://github.com/lauerfab/MLweb/blob/bf9a67c03372dcf21aade28984806d2486e4a665/lalolab/src/linalg.js#L6947

Calling svd on this matrix (it's centered) will trigger the infinite loop:

[
  [-0.5, -0.5, 1, -1.5, 1.5, 0], 
  [0.5, 0.5, -1, 1.5, -1.5, 0]
]

but this one won't:

[
  [-0.5, -0.5, 1, -1.5, 1.5, 0],  
  [0.5, 0.5, -1, 1.5, -1.5, 1]
]

I tried a few different examples and not every centered matrix triggers this issue, but I haven't found a non-centered matrix that triggers the problem.

lauerfab commented 6 years ago

Thanks for pointing this out. I just fixed some indexing problem in a specific case (it seems that what really triggered it is the last column of zeros). It now works on your example. Can you try with some other examples and let me know if you still find this issue?

mooreryan commented 6 years ago

Thanks for the fix! The original problem seems to be okay now.

I tried a few more examples and I found one that wasn't working. I ran JSON.stringify on the matrix and put that output into a file so you could get the data.

centered_transposed_longley_data.txt

If it helps you at all, the matrix comes from the longley data set in R. It is transposed then centered.

lauerfab commented 6 years ago

Fixed again. There was a typo (* instead of +) in the code. It seems to be ok now, but let me know if it works on your specific data. PS: if you intend to do regression with the builtin functions, note that the observations in the matrix X should be given as rows and the variables as columns, so your longley matrix should not be transposed in this case.