Skip to content

[MRG] Spectral_embedding mistake correction#8217

Closed
devanshdalal wants to merge 2 commits intoscikit-learn:masterfrom
devanshdalal:devanshdalal/bug-fix-#8189
Closed

[MRG] Spectral_embedding mistake correction#8217
devanshdalal wants to merge 2 commits intoscikit-learn:masterfrom
devanshdalal:devanshdalal/bug-fix-#8189

Conversation

@devanshdalal
Copy link
Copy Markdown
Contributor

@devanshdalal devanshdalal commented Jan 19, 2017

Fixes #8129

Please provide suggestions for this pull request.

I hope more strong unit_tests might be needed,

@jnothman
Copy link
Copy Markdown
Member

Please use a more descriptive PR title

@devanshdalal devanshdalal changed the title [MRG] fixes #8189 [MRG] Spectral_embedding correction Jan 20, 2017
@devanshdalal devanshdalal changed the title [MRG] Spectral_embedding correction [MRG] Spectral_embedding mistake correction Jan 20, 2017
tol=eigen_tol, v0=v0)
embedding = diffusion_map.T[n_components::-1] * dd
embedding = diffusion_map.T[n_components::-1]
if(norm_laplacian):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if norm_laplacian:

largest=False)
embedding = diffusion_map.T * dd
embedding = diffusion_map.T
if(norm_laplacian):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if norm_laplacian:

lambdas, diffusion_map = eigh(laplacian)
embedding = diffusion_map.T[:n_components] * dd
embedding = diffusion_map.T
if(norm_laplacian):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if norm_laplacian:

lambdas, diffusion_map = lobpcg(laplacian, X, tol=1e-15,
largest=False, maxiter=2000)
embedding = diffusion_map.T[:n_components] * dd
embedding = diffusion_map.T
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why removing the :n_components?

@agramfort
Copy link
Copy Markdown
Member

@devanshdalal any example is clearly impacts where we see that bug fix has a practical positive implication?

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