Skip to content

[MRG] Accelerate examples/manifold/plot_compare_methods.py #21635

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 8, 2022
Merged

[MRG] Accelerate examples/manifold/plot_compare_methods.py #21635

merged 2 commits into from
Feb 8, 2022

Conversation

yuanx749
Copy link
Contributor

Reference Issues/PRs

#21598

What does this implement/fix? Explain your changes.

Reduce the time from 14 sec to 8 sec.

  • For MDS, decrease max_iter from 100 to 50.
  • For t-sne, Decrease n_iter from 1000 (default) to 250, and decrease perplexity from 30 (default) to 10.
  • Set random_state for reproducibility.

Before:
before

After:
after

Any other comments?

TomDLT
TomDLT approved these changes Nov 11, 2021
View reviewed changes
Copy link
Member

@TomDLT TomDLT left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@TomDLT TomDLT mentioned this pull request Nov 11, 2021
Closed
41 tasks
ogrisel
ogrisel reviewed Nov 12, 2021
View reviewed changes
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

Personally, I find that the result on t-SNE to be qualitatively too degraded for this change to be acceptable.

I think we should focus our effort to improve the speed of t-SNE in scikit-learn instead (e.g. maybe through #20254 and follow-up).

+1 for the change in MDS params (it seems qualitatively better with early stopping...)

@yuanx749
Copy link
Contributor Author

Since perplexity is a measure of the effective number of neighbors, I changed it to 10. It seems to affect too much. Here is the result using the default perplexity=30.
after2

@adrinjalali
Copy link
Member

@yuanx749 we can merge this if you change the t-sne parameters to something that the results look not too degraded.

@yuanx749 yuanx749 force-pushed the plot_compare_methods branch from dd0cd49 to bc4aaed Compare February 4, 2022 16:17
@yuanx749
Copy link
Contributor Author

yuanx749 commented Feb 4, 2022

@adrinjalali I changed back the perplexity. Do you think the result looks OK? (See image above or here)

@adrinjalali adrinjalali merged commit 5900e26 into scikit-learn:main Feb 8, 2022
@yuanx749 yuanx749 deleted the plot_compare_methods branch February 8, 2022 12:22
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Feb 9, 2022
@yuanx749 @glemaitre
DOC Accelerate examples/manifold/plot_compare_methods.py (scikit-lear…
bca6e90 
…n#21635)

* Accelerate plot_compare_methods.py

* Change tsne perplexity
ilivans pushed a commit to ilivans/scikit-learn that referenced this pull request Feb 10, 2022
@yuanx749
DOC Accelerate examples/manifold/plot_compare_methods.py (scikit-lear…
988cafc 
…n#21635)

* Accelerate plot_compare_methods.py

* Change tsne perplexity
thomasjpfan pushed a commit to thomasjpfan/scikit-learn that referenced this pull request Mar 1, 2022
@yuanx749 @thomasjpfan
DOC Accelerate examples/manifold/plot_compare_methods.py (scikit-lear…
0a210d6 
…n#21635)

* Accelerate plot_compare_methods.py

* Change tsne perplexity
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel left review comments

@adrinjalali adrinjalali adrinjalali approved these changes

@TomDLT TomDLT TomDLT approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@yuanx749 @adrinjalali @ogrisel @TomDLT