8000 DOC Increase execution speed of plot_sgd_comparison · Pull Request #21610 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC Increase execution speed of plot_sgd_comparison #21610

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 8 commits into from Dec 9, 2021
Merged

DOC Increase execution speed of plot_sgd_comparison #21610

merged 8 commits into from Dec 9, 2021

Conversation

ghost
Copy link
@ghost ghost commented Nov 9, 2021

#21598 @sply88 @adrinjalali @cakiki

Adapted a few things in the module examples/linear_model/plot_sgd_comparison to make it a bit faster:

  • Tuned down the number of rounds/loops by 25 %
  • Adapted the max_iter parameter to a point where execution is still fast but parameter doesn't kill the optimization process
  • Added the option of a warm_start to a bunch of classifiers

@adrinjalali
Copy link
Member

@sveneschlbeck it'd be useful to post the output of the example (text, and image if it produces any) before and after your change, and the time it takes to run on your machine (you can do time python /path/to/example.py to get the times in linux, and mac I think.

there's also failing lint tests in your PR, you should enable black on your clone. See here for more info, you can also activate git hooks on your clone which would do this automatically.

@ghost
Copy link
Author
ghost commented Nov 9, 2021

@adrinjalali I ran test using the datetime.datetime.now() measurement function and ran both scripts (original and mine) in an interactive kernel with the exact same conditions. My version took around half the time:
before
after

@adrinjalali
Copy link
Member

@ogrisel @mblondel you've had opinions on this example in the past, it'd be nice if you could check this.

@adrinjalali adrinjalali mentioned this pull request Nov 10, 2021
41 tasks
@adrinjalali adrinjalali changed the title Increase execution speed of plot_sgd_comparison through parameter adaptions Increase execution speed of plot_sgd_comparison.py through parameter adaptions Nov 12, 2021
Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @sveneschlbeck !

I think that setting warm_start=True changes the narrative of the example. The model trained on a previous round will influence the model in the next.

Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
@ghost
Copy link
Author
ghost commented Nov 23, 2021

@thomasjpfan Agreed, we also had a similar 8000 discussion at another example and concluded that it may change the example's character. Also, some examples are never run multiple times in a row, making the param toothless. Will remove warm_start.

Thanks for the input!

@adrinjalali
Copy link
Member

Please apply black on your code to pass the linters in the CI

@ghost
Copy link
Author
ghost commented Nov 24, 2021

@adrinjalali Will do

@thomasjpfan
8000 Copy link
Member

The error on circleci should be fixed by syncing with main.

Copy link
Member
@thomasjpfan thomasjpfan left a comment

Choose a reason for hiding this comment

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

LGTM

@thomasjpfan thomasjpfan changed the title Increase execution speed of plot_sgd_comparison.py through parameter adaptions DOC Increase execution speed of plot_sgd_comparison.py through parameter adaptions Dec 9, 2021
@thomasjpfan thomasjpfan changed the title DOC Increase execution speed of plot_sgd_comparison.py through parameter adaptions DOC Increase execution speed of plot_sgd_comparison Dec 9, 2021
@thomasjpfan thomasjpfan merged commit c29fe81 into scikit-learn:main Dec 9, 2021
@ghost ghost deleted the speed_increased_example_modlinear branch December 9, 2021 18:04
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Dec 24, 2021
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
glemaitre pushed a commit that referenced this pull request Dec 25, 2021
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0