Skip to content

DOC add links to neural network examples #26935

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

Conversation

punndcoder28
Copy link
Contributor

@punndcoder28 punndcoder28 commented Jul 29, 2023

Reference Issues/PRs

Adds links to neural-networks examples as mentioned in #26927

What does this implement/fix? Explain your changes.

Adds links to auto-generated examples for classes MLPClassifier, BernoulliRBM, SGDOptimizer and AdamOptimizer

Classes and files updated with examples link

  1. MLPClassifier (sklearn/neural_network/_multilayer_perceptron.py)
  2. BernoulliRBM (sklearn/neural_network/_rbm.py)
  3. SGDOptimizer (sklearn/neural_network/_stochastic_optimizers.py)
  4. AdamOptimizer (sklearn/neural_network/_stochastic_optimizers.py)

@punndcoder28 punndcoder28 changed the title docs(neural_networks): add links to neural network examples DOC add links to neural network examples Jul 29, 2023
@github-actions
Copy link

github-actions bot commented Jul 29, 2023

✔️ Linting Passed

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

Generated for commit: f09c077. Link to the linter CI: here

@adrinjalali
Copy link
Member

Please link all files for which you're creating links in the description, and limit the PR to a single class (or group of a class if there are classification and regression for the same algorithm)

@punndcoder28
Copy link
Contributor Author

Will update the PR description to contain all the files for which the links have been created.

limit the PR to a single class (or group of a class if there are classification and regression for the same algorithm)

Does this mean that separate PRs should be created for BernoulliRMB and a single PR for SGDOptimizer and AdamOptimizer since they are both related?

@adrinjalali
Copy link
Member

To simplify the PR, you can take a single example and then put link in different places wherever relevant. then it'd be okay if it touches multiple classes.

@punndcoder28
Copy link
Contributor Author

punndcoder28 commented Aug 1, 2023

@adrinjalali on further digging through the code, I notice that the MLPClassifier class has been referenced in multiple examples, in such a scenario what should be done?

I double checked the references for BernoulliRBM and only the plot_rbm_logistic_classification.py references them

I was also not able to find any other refs for SGDOptimizer and AdamOptimizer apart from their mention in sklearn/neural_network/_stochastic_optimizers.py.

Please let me know if I am missing more references.

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

You can keep them all here, ended up reviewing as is. Thank you :)

@punndcoder28 punndcoder28 force-pushed the link_examples_neural_networks branch from 906d0a0 to 8061e7f Compare August 27, 2023 06:03
@punndcoder28
Copy link
Contributor Author

Thanks for the comments @adrinjalali I've resolved all of them with the latest commit

Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Also liter's failing.

@punndcoder28
Copy link
Contributor Author

I've updated the code as per the new suggestions

@adrinjalali linter is failing since we use assert type(expected_report["setosa"]["precision"]) == float to check types and it is recommending we use isinstance(). Can I open a PR which aims to fix this linting issues?

@punndcoder28 punndcoder28 force-pushed the link_examples_neural_networks branch from 99b67ba to add3131 Compare August 29, 2023 16:07
For a comparison between Adam optimizer and SGD, see
:ref:`sphx_glr_auto_examples_neural_networks_plot_mlp_training_curves.py`
Copy link
Member

Choose a reason for hiding this comment

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

missing a . at the end of all these sentences.

@marenwestermann
Copy link
Member

@punndcoder28 would you like to continue working on this PR?

@punndcoder28
Copy link
Contributor Author

Apologies, the comments did not notify me. Will update the PR again

@punndcoder28 punndcoder28 force-pushed the link_examples_neural_networks branch from 81472b1 to cdbe78f Compare January 6, 2024 03:39
@adrinjalali adrinjalali merged commit 88e1b82 into scikit-learn:main Feb 22, 2024
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.

3 participants