DOC Merge plot_svm_margin.py and plot_separating_hyperplane.py into plot_svm_hyperplane_margin.py #31045
Conversation
There was a problem hiding this comment.
Hi @SwathiR1999,
thank you for your patience and for your efforts to merge the examples. It looks very good and I like the design of the plots. The plots all have heading and legend and are easy on the eye.
I think we could add some information and I have left you some suggestions on that, too.
When I build the docs locally, I cannot click to open it and also there is some text overflow in the background. I believe this is because of a rending issue in the heading, and I have added a suggestions for a fix.
What is lacking is to remove the old examples and to add redirections from both old links to the new example link in the doc/conf.py file. You can have a look at #30028 to see how to add the redirections.
You need to delete all links to the old examples as well (and replace it with the new one if suitable). There are some, probably too many, links to plot_separating_hyperplane_unbalanced currently.
| - **Small C (e.g., 0.05)**: | ||
| - Allows some misclassifications, resulting in a wider margin. | ||
| - **Moderate C (e.g., 1)**: | ||
| - Balances classification accuracy and margin width. | ||
| - **Large C (e.g., 1000)**: | ||
| - Prioritizes classifying all points correctly, leading to a narrower margin. |
There was a problem hiding this comment.
I think while reworking the example, we could add some more information.
Do you think you could add some suggestion of when to use a small, a moderate or a high C value in a practical use case?
We could also talk about overfitting and underfitting in this context.
Also, we could talk about miss-classification / error for different margins.
I think this could be added after the code for the plot using the # % syntax to make this example more notebook-like. (Also have a look at #30028 for a good example on the notebook style.)
| """ | ||
| ========================================================================= | ||
| SVM: Effect of Regularization (C) on Maximum Margin Separating Hyperplane | ||
| ======================================================================== |
There was a problem hiding this comment.
| ======================================================================== | |
| ========================================================================= |
The lacking "=" leads to the title not being rendered correctly. Adding "=" should fix it.
I also hope that this fixes the problem with the example not being clickable from the main examples page and the text overflow there (see other comment).
| clf.fit(X, y) | ||
|
|
||
| plt.subplot(1, 3, i) | ||
| plt.scatter(X[:, 0], X[:, 1], c=y, s=30, cmap=plt.cm.Paired, edgecolors="k") |
There was a problem hiding this comment.
Please add axis labels. Something simple like "Feature 1" and "Feature 2" would be enough.
| s=100, | ||
| linewidth=1.5, | ||
| facecolors="none", | ||
| edgecolors="r", |
There was a problem hiding this comment.
What do you think about having a different color for miss-classified samples?
…nto doc_addlink
|
Hi @StefanieSenger , thanks for the feedback!
Just to clarify - should I also apply similar changes (merge/update/remove references) to |
Oh, my bad. I didn't see there were |
…nto doc_addlink
- Modified \`doc/conf.py\` and \`doc/modules/svm.rst\` to update documentation and add example reference.\n- Removed outdated example: \`examples/svm/plot_svm_margin.py\`.
|
Hi @StefanieSenger, I’ve implemented your suggestions. I wanted to check with you on one small point: While It is currently referenced early in the SVM documentation, which aligns well with its simplicity. The new merged example, on the other hand, focuses more on the effect of Would it be alright to:
Looking forward to your thoughts! |
Reference Issues/PRs
References #30621
What does this implement/fix? Explain your changes.
plot_svm_margin.pyandplot_separating_hyperplane.py) into a single fileplot_svm_hyperplane_margin.py.Any other comments?
This PR addresses the feedback from #30975, which was closed.
@StefanieSenger Looking forward to your feedback. Thanks!