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

Result contains NaN for some matrices #5

Open
hasanbalci opened this issue Feb 20, 2020 · 8 comments
Open

Result contains NaN for some matrices #5

hasanbalci opened this issue Feb 20, 2020 · 8 comments

Comments

@hasanbalci
Copy link

I am considering to use your implementation, but during testing I realized that the result (especially u and q) of some matrices contains NaN values.

Some examples are:

input: [[-896, -896], [-19, -19]]
output:
u = [[NaN, NaN], [NaN, NaN]]
q = [0, NaN]
v = [[0.7071067811865476, -0.7071067811865475], [-0.7071067811865474, -0.7071067811865475]]

input: [[0, 0],[-1560.116, -2789.99]]
output:
u = [[NaN, NaN], [NaN, NaN]]
q = [0, NaN]
v = [[0.8728097060016351, -0.48806066949646665], [-0.48806066949646654, -0.8728097060016349]]

Thanks!

@DamianCz
Copy link

DamianCz commented Apr 4, 2020

There are some divided by zero in implementation I can provide fix @danilosalvati if you want.

@danilosalvati
Copy link
Owner

danilosalvati commented Apr 5, 2020

Sorry @hasanbalci I completely forgot to answer your issue... At a first look it seem your bug is caused from eps parameter... I tried with runkit your first example with Number.MIN_VALUE as eps and it works

const svdJs = require("svd-js")
const a = [[-896, -896], [-19, -19]]
const { u, v, q } = svdJs.SVD(a, true, true, 1e-15)
console.log(u)
console.log(v)
console.log(q)

// output:
// [[0.02120059107367143, -0.9997752422110318], [-0.9997752422110316,0.021200591073671428]]
// [[-0.7071067811865475, 0.7071067811865476], [0.7071067811865475, 0.7071067811865474]]
// [0, 1267.4202144513872]

@DamianCz Where did you found those divided by zero in the implementation? Anyway PRs are always welcome 😄

@hasanbalci
Copy link
Author

@danilosalvati It seems that adding eps parameter as 1e-15 is working in my first example but not in the second one.

@DamianCz
Copy link

@danilosalvati it is safe to check all divided places . I will add PR when I will find time

@pygabc1
Copy link

pygabc1 commented Oct 9, 2020

There are some divided by zero in implementation I can provide fix @danilosalvati if you want.

Hi, DamianCz, did you fix "ZeroDivisionError: float division by zero"?
Many lines occurred these problems, and it might result in the left singular vector(matrix) with "all zero components".
For example, for
c = f/z , I modified to
c = (f/z if z != 0 else 0) (for python)

@DamianCz
Copy link

DamianCz commented Oct 12, 2020

@pygabc1 I have added function and replace all usage of / with this function. Will add PR this week.

function dividedValue(divident, divisor) {
        if (divisor === 0) {
            return 0;
        }

        return divident / divisor;
    }

@jee7
Copy link

jee7 commented May 4, 2023

Did this ever get fixed?
I have the same issue right now with version 1.1.1.

var result = SVDJS.SVD([
    [1, 2, 3],
    [2, 4, 6],
    [3, 6, 9],
], true, true, 1e-15);

The expected output for q would be [14, 0, 0], but I'm getting [0, NaN, 0] instead.

I tried the solution that replaces all the divisions with the function shown by @DamianCz. Yes, the NaN values are gone, but q will then just be [0, 0, 0] not the expected [14, 0, 0].

@donhatch
Copy link

I ran into this too. Another example:

input: [[0.018314685707290043, 0.1340867554784432],
        [0.13408675547842389,  0.98168531429271]]
output: {
  u: [[NaN,NaN],[NaN,NaN]],
  q: [0,NaN],
  v: [[0.9908003402768454,-0.1353317616352032],
      [-0.1353317616352032,-0.9908003402768454]]
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants