8000 Increase execution speed of plot_cluster_comparison.py by Iglesys347 · Pull Request #21624 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

Increase execution speed of plot_cluster_comparison.py #21624

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

Merged
merged 7 commits into from
Mar 10, 2022
Merged

Increase execution speed of plot_cluster_comparison.py #21624

merged 7 commits into from
Mar 10, 2022

Conversation

Iglesys347
Copy link
Contributor
@Iglesys347 Iglesys347 commented Nov 10, 2021

Reference Issues/PRs

References #21598

What does this implement/fix? Explain your changes.

Reduced the number of parameters (divided by 3) in order to increase the execution speed.

Here is the graphs with the base method (1500 parameters) :

base_figure

And here the ones with the new method (500 parameters):

new_figure2

Finally the graphs with 500 parameters and tunned parameters

fig_new

We cannot reduce more the number of parameters otherwise we would not be able to see the scalability.

Any other comments?

@adrinjalali adrinjalali mentioned this pull request Nov 12, 2021
41 tasks
@adrinjalali
Copy link
Member

The optics results don't look very good now, could you please play with its parameters so that you get a similar result as before?

@Iglesys347
Copy link
Contributor Author

Yes, I agree, I'm working on it.

@Iglesys347
Copy link
Contributor Author

@adrinjalali I played with the parameters and the results looks better to me, what do you think ?

@adrinjalali
Copy link
Member

@Iglesys347 would you mind merging with the latest main? Sorry for the delay, I've got time to check this now.

@Iglesys347
Copy link
Contributor Author

@adri 8000 njalali I merged the latest main and the checks fails ; but they did not fail before and I didn't change anything in the code.

@thomasjpfan
Copy link
Member

The CI error is unrelated to your change and is being addressed in #22381

@Iglesys347
Copy link
Contributor Author

Hello @adrinjalali I merged the latest changes from main and the pipeline looks ok, I think we are good to merge ?

@adrinjalali
Copy link
Member

Yep, this is perfect, thank you @Iglesys347

@adrinjalali adrinjalali merged commit 5b04fe6 into scikit-learn:main Mar 10, 2022
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Apr 6, 2022
…arn#21624)

* Reduced n_samples to increse speed

* ENH imporved visuals of Optic method for the first two dataset, still working on the other datasets

* ENH improved visuals for Optic method by tuning parameters

* Linting with black
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
0