tompollard / sammon

Sammon mapping in Python
32 stars 18 forks source link

all output are nan?! #1

Closed Hualin closed 7 years ago

Hualin commented 9 years ago

hey, thanks for you code. but I run the sammontest.py, both raw and distance output are nan, because E-E_new array is all nan.

at first glance i am not sure where to fix it, since there Dinv but you catch the inf error and replace it with 0.

tompollard commented 9 years ago

Hi Hualin, thanks for your message. I haven't looked at the code in a while, but I'll check this out when I get a chance. The code could do definitely do with some cleaning up.

The functionality was based on a Matlab function, which I remember would return NaNs in the following circumstances:

Do either of these issues apply here? i.e. please could you try removing (1) all NaNs from your input array and (2) any duplicate data points from your input array and let me know if this helps?

Hualin commented 9 years ago

Cool, thanks for your fast reply. I guess sum of E_new or E is NaN because there was a value NaN in the array of E. Perhaps, delta = D-d.

I believe I should do some cleanup routine on distance matrix for D and d, it should fix something. I will be back here.

Hualin commented 9 years ago

Hi there. I believe I got it fixed. Though I drop something:

  1. dropinputdist = "distance", always give vector space. (it can be hacked if one need to give distance matrix directly)
  2. drop euclid function, use scipy.spetial.distance.cdist instead.
  3. change some naming conversions: x => X, y=>Y, D=>xD, d=>yD, Dinv=>xDinv, dinv=>yDinv (the code below use these names)
  4. make some change about xDInv build: yD = distance.cdist(Y,Y)
np.fill_diagonal(xD, 1)
np.fill_diagonal(yD, 1)

xDinv = 1.0/xD
yDinv = 1.0/yD
np.fill_diagonal(xDinv, 0)
np.fill_diagonal(yDinv, 0)
xDinv[np.isnan(xDinv)] = 0
yDinv[np.isnan(yDinv)] = 0
xDinv[np.isinf(xDinv)] = 0
yDinv[np.isinf(yDinv)] = 0

and somehow. those changes make error gone! I didn't touch the optimisation process.

cheers,

tompollard commented 9 years ago

Thanks Hualin - please could you put the suggested changes into a pull request?

tompollard commented 7 years ago

This should have been addressed in the merged pull request.