10000 DOC fix random_state in example for reproducibility cont'd by TamaraAtanasoska · Pull Request #27238 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

DOC fix random_state in example for reproducibility cont'd #27238

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 9 commits into from
Sep 7, 2023
Merged

DOC fix random_state in example for reproducibility cont'd #27238

merged 9 commits into from
Sep 7, 2023

Conversation

TamaraAtanasoska
Copy link
Contributor

Reference Issues/PRs

Fixes a part of #17568. This PR together with #27153 contains all the files necessary to close the issue.

What does this implement/fix? Explain your changes.

This PR introduces minor changes in three files:

  • examples/applications/plot_stock_market.py
  • examples/manifold/plot_compare_methods.py
  • examples/manifold/plot_manifold_sphere.py
  • examples/miscellaneous/plot_kernel_approximation.py
  • examples/svm/plot_rbf_parameters.py

Most of the changes are minor. Just one image has a noticeable difference when the seed is introduced, both images are posted in the first comment of this PR to be reviewed. All the other images maintain the visual impression.

One file was found to not exist any more, and several were marked as completed in #17568 (comment) as the randomness was already removed.

Any other comments?

@glemaitre @adrinjalali please take a look 👋

@github-actions
Copy link
github-actions bot commented Aug 30, 2023 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 28f408f. Link to the linter CI: here

@TamaraAtanasoska
Copy link
Contributor Author

Changes to the first image of the examples/miscellaneous/plot_kernel_approximation.py file. @glemaitre @adrinjalali please let me know if this is close enough or I should change the seed.

Before

Screen Shot 2023-08-30 at 14 52 35

After

Screen Shot 2023-08-30 at 14 46 38

@TamaraAtanasoska TamaraAtanasoska mentioned this pull request Aug 30, 2023
Closed
@TamaraAtanasoska TamaraAtanasoska changed the title FIX Create a cosistent image via random_state additions in several example files FIX Create a cosistent image via random_state additions in several example files (2) Aug 30, 2023
@glemaitre glemaitre changed the title FIX Create a cosistent image via random_state additions in several example files (2) DOC fix random_state in example for reproducibility cont'd Sep 7, 2023
@glemaitre glemaitre self-requested a review September 7, 2023 09:42
@TamaraAtanasoska @glemaitre
Update examples/applications/plot_stock_market.py
9a6ad7f 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@TamaraAtanasoska @glemaitre
Update examples/miscellaneous/plot_kernel_approximation.py
410dce6 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@glemaitre
Copy link
Member

I check all images and everything is fine.
We cannot have deterministic image when we are plotting the timing :).
Once the last comments are addressed I can merge this PR.

TamaraAtanasoska and others added 2 commits September 7, 2023 12:01
@TamaraAtanasoska @glemaitre
Update examples/svm/plot_rbf_parameters.py
ff53680 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@TamaraAtanasoska @glemaitre
Update examples/svm/plot_rbf_parameters.py
28f408f 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
@glemaitre glemaitre merged commit 51ca717 into scikit-learn:main Sep 7, 2023
@glemaitre
Copy link
Member

Cool. Merging this one.
Since it was the last batch of changes, I am going to close the issue. Thanks a lot @TamaraAtanasoska to have tracked this issue down.

@TamaraAtanasoska
Copy link
Contributor Author

Cool. Merging this one. Since it was the last batch of changes, I am going to close the issue. Thanks a lot @TamaraAtanasoska to have tracked this issue down.

Thanks to both you and @adrinjalali for the reviews! I learned a lot :)

@TamaraAtanasoska TamaraAtanasoska deleted the remove-randomness-3 branch September 7, 2023 10:03
glemaitre added a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
@TamaraAtanasoska @glemaitre
DOC fix random_state in example for reproducibility cont'd (scikit-le…
f4bae4b 
…arn#27238)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
@TamaraAtanasoska @glemaitre @jeremiedbb
DOC fix random_state in example for reproducibility cont'd (#27238)
b10c8ab 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
@TamaraAtanasoska @glemaitre @REDVM
DOC fix random_state in example for reproducibility cont'd (scikit-le…
5e8011f 
…arn#27238)

Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre left review comments

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

Successfully merging this pull request may close these issues.
2 participants
@TamaraAtanasoska @glemaitre
0