MLweb icon indicating copy to clipboard operation
MLweb copied to clipboard

Infinite loop in svd function

Open mooreryan opened this issue 8 years ago • 3 comments

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.

mooreryan avatar Mar 13 '18 04:03 mooreryan

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?

lauerfab avatar Mar 13 '18 11:03 lauerfab

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.

mooreryan avatar Mar 13 '18 15:03 mooreryan

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.

lauerfab avatar Mar 14 '18 15:03 lauerfab