sammon icon indicating copy to clipboard operation
sammon copied to clipboard

added jupyter notebook and used updated sammon function

Open nsadawi opened this issue 7 years ago • 4 comments

nsadawi avatar May 05 '17 14:05 nsadawi

Thanks for the pull request, and I think the addition of a Jupyter Notebook is a great idea. Before merging the Notebook, I think the following updates would be helpful for users:

  • some explanatory text to walk the user through the example
  • using the function provided in the python module (https://github.com/tompollard/sammon/blob/master/sammon.py), rather than redefining the function.

Is it possible that you could look at these points?

tompollard avatar Jan 04 '18 12:01 tompollard

Hi @tompollard .. yes I'll have a look Noureddin

nsadawi avatar Jan 04 '18 13:01 nsadawi

Oki doki .. I've update the notebook import the sammon function from sammon.py Please observe I am still using the code updated by @Irene-GM as your code shows a value of NaN for E I also changed this line s_reshape = s.reshape(2,len(s)/2).T to s_reshape = s.reshape(2,len(s)//2).T to make sure integer division was done .. otherwise numpy complains

Just pushed my latest version

Best wishes, Noureddin

nsadawi avatar Jan 04 '18 14:01 nsadawi

@nsadawi You shouldn't use the updated Sammon, because it doesn't do things right. Sammon should not be applied to cases where dissimilarity == 0 for some samples. Please check https://github.com/tompollard/sammon/pull/8 for an updated version (which fixes this and other bugs as well)

devernay avatar Jun 26 '19 18:06 devernay