Skip to content

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

Merged
adrinjalali merged 2 commits intoscikit-learn:mainfrom
yuanx749:plot_compare_methods
Feb 8, 2022
Merged

[MRG] Accelerate examples/manifold/plot_compare_methods.py#21635
adrinjalali merged 2 commits intoscikit-learn:mainfrom
yuanx749:plot_compare_methods

Conversation

@yuanx749
Copy link
Copy Markdown
Contributor

@yuanx749 yuanx749 commented Nov 11, 2021

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

yuanx749 commented Nov 12, 2021

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
Copy Markdown
Member

adrinjalali commented Feb 4, 2022

@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
Copy Markdown
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

+1 more reviewer

@TomDLT TomDLT TomDLT approved these changes

Reviewers whose approvals may not affect merge requirements

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