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

Embedding doc #424

Merged
merged 4 commits into from
Apr 26, 2018
Merged

Embedding doc #424

merged 4 commits into from
Apr 26, 2018

Conversation

jetfuel
Copy link
Collaborator

@jetfuel jetfuel commented Apr 25, 2018

Reveal the high dimensional tab
Add function documentation
Improve loading animation algorithm
Use the in-house PCA function
Use the free T-SNE function

Resolve #407

@jetfuel jetfuel self-assigned this Apr 25, 2018
nickyfantasy
nickyfantasy previously approved these changes Apr 25, 2018
@@ -292,9 +295,19 @@ def text(self, tag):
return self.writer.new_text(tag)

def embedding(self):
"""
Create an embedding writer that used to write
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is used to

Create an embedding writer that used to write
embedding data.

:return: A embedding writer to record embedding data
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An embedding

@@ -393,3 +392,23 @@ def _handler(key, func, *args, **kwargs):
return data

return _handler


# A simple PCA implementaiton to do the dimension reduction.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we comment methods like this:

def kos_root():
    """Return the pathname of the KOS root directory."""
    global _kos_root
    if _kos_root: return _kos_root
    ...

cov = np.cov(x, rowvar=False)

# Get eigenvectors and eigenvalues from the covariance matrix
eigvals, eigvecs = np.linalg.eig(cov)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a math guy, Jeff! But do we need to do it ourselves? Moreover, SVD is more stable than eigenvector in terms of computing principle components.

Check here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason behind it so we don't have to import another pip package to do the calculation.
And actually the TSNE file already has a clean PCA implementation. and I could just use that.

@@ -0,0 +1,184 @@
#
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should give the original author a citation 😀

https://github.com/bhauman/neurpy/blob/master/tsne.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did give the author credit, I left my comments under the author's comments.

daming-lu
daming-lu previously approved these changes Apr 25, 2018
@jetfuel jetfuel dismissed stale reviews from daming-lu and nickyfantasy via ad121f3 April 26, 2018 00:04
@jetfuel jetfuel merged commit 2642aab into PaddlePaddle:develop Apr 26, 2018
@jetfuel jetfuel deleted the embeddingDoc branch April 26, 2018 00:25
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

Successfully merging this pull request may close these issues.

3 participants