-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC Notebook style and enhanced descriptions in SVM kernels example #26805
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
DOC Notebook style and enhanced descriptions in SVM kernels example #26805
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can fix the math with this patch:
diff --git a/examples/svm/plot_svm_kernels.py b/examples/svm/plot_svm_kernels.py
index 3c4772fa3..e475d6936 100644
--- a/examples/svm/plot_svm_kernels.py
+++ b/examples/svm/plot_svm_kernels.py
@@ -210,8 +210,8 @@ plot_training_data_with_decision_boundary("linear")
# The polynomial kernel changes the notion of similarity. The kernel function
# is defined as:
#
-# .. math:: K(\mathbf{x}_1, \mathbf{x}_2) = (\gamma \cdot \
-# .. math:: \mathbf{x}_1^\top\mathbf{x}_2 + r)^d
+# .. math::
+# K(\mathbf{x}_1, \mathbf{x}_2) = (\gamma \cdot \mathbf{x}_1^\top\mathbf{x}_2 + r)^d
#
# where :math:`{d}` is the degree (`degree`) of the polynomial, :math:`{\gamma}`
# (`gamma`) controls the influence of each individual training sample on the
@@ -238,8 +238,9 @@ plot_training_data_with_decision_boundary("poly")
# similarity between two data points in infinite dimensions and then approaches
# classification by majority vote. The kernel function is defined as:
#
-# .. math:: K(\mathbf{x}_1, \mathbf{x}_2) = \exp\left(-\gamma \cdot \
-# .. math:: {\|\mathbf{x}_1 - \mathbf{x}_2\|^2}\right)
+# .. math::
+# K(\mathbf{x}_1, \mathbf{x}_2) = \exp\left(-\gamma \cdot \
+# {\|\mathbf{x}_1 - \mathbf{x}_2\|^2}\right)
#
# where :math:`{\gamma}` (`gamma`) controls the influence of each individual
# training sample on the decision boundary.
@@ -258,8 +259,9 @@ plot_training_data_with_decision_boundary("rbf")
# **************
# The sigmoid kernel function is defined as:
#
-# .. math:: K(\mathbf{x}_1, \mathbf{x}_2) = \tanh(\gamma \cdot \\
-# .. math:: \mathbf{x}_1^\top\mathbf{x}_2 + r)
+# .. math::
+# K(\mathbf{x}_1, \mathbf{x}_2) = \tanh(\gamma \cdot \
+# \mathbf{x}_1^\top\mathbf{x}_2 + r)
#
plot_training_data_with_decision_boundary("sigmoid")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@glemaitre do you wanna have a look?
@StefanieSenger could you please also add a link to this example in the SVM api docs? |
@adrinjalali Sure. Where exactly do you think of putting it? It is shown automatically at the bottom of sklearn.svm.SVC. Or do you mean the user guide? |
The issue with the ones automatically shown on the bottom of API docs is that it's really not clear what example is relevant for what. Adding the link explicitly in the API doc would make these much more discoverable. I'd add it to the docstring of |
Yep I will have a look now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I propose some simplification regarding the plotting part.
I will check the narrative in early afternoon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with the rest of the example.
Thanks for your review @glemaitre. I have substituted the old matplotlib plotting by our |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of matplotlib simplifications.
Otherwise LGTM.
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Thank you, @ArturoAmorQ for your comments. You are right with your procaution regarding the margins being at a distance from the boundary equal to the C param on the linear kernel. I was assuming something empirically, that might as well have been coincidence. I have deleted that information to be sure. I have modified the Unfortunately I have pushed some wrong files along with the changes (was late yesterday). I will take care of this, but I am not sure if I have time for this before Wednesday. I have turned this PR into a draft in the meantime. |
adf1006
to
33a9b3a
Compare
I've now deleted the accidentally pushed files and I hope I did it in the cleanest possible way. Sorry for adding confusion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some final nitpicks :)
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
…it-learn into examples_svm_kernels
Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the PR @StefanieSenger! 🎉
On a side note, this is going to be my very first merge on scikit-learn :)
Yeahhhh, let's celebrate, @ArturoAmorQ 🎉🎉🎉 |
Thank you, @StefanieSenger! And congratulations, @ArturoAmorQ! 🎉 With great power comes great responsibility. 🙂 |
What does this implement/fix? Explain your changes.
This PR suggests to introduce notebook style formatting and enhanced descriptions in the SVM kernels example,
Any other comments?
A few issues:
Looking forward to your comments. :)