-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
DEP start to raise warning with inconsistent combination of hyperparameters in SVM #19630
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
base: main
Are you sure you want to change the base?
Conversation
Let me know if I should add a |
if self.kernel == 'linear': | ||
warnings.warn( | ||
"Setting 'gamma' when using 'linear' kernel may raise a " | ||
"`ValueError` starting in version 1.1 (renaming of 0.26).", |
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 would keep the usual cycle: if we start warning in 1.0, we should start erroring in 1.2 (n+0.2).
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.
Thanks for the pointer, I wasn't familiar with the usual cycle so I assumed it would be n+0.1
:) I'll start making changes when a consensus is reached in #19614.
Before to review, I would like that we settle on what do for all parameters combination. It will be discussed in the original issue #19614 |
The issue seems to have moved forward a bit, I think we can move forward in this PR @PGijsbers |
While I would still be very happy to contribute, I am very busy right now, so it might take a while for me to get to this (likely more than a month). If anyone else wants to take this, that is good with me. If not, then I should come around to it eventually. |
References #19614.
What does this implement/fix? Explain your changes.
Gives a
FutureWarning
when a user configures aLibSVM
learner with 'linear'kernel
but also non-defaultgamma
.previous output:
current output:
Any other comments?
Went with a
FutureWarning
instead of theDeprecationWarning
as discussed in the issue, because:DeprecationWarning
is ignored by default (unless it's code from__main__
),gamma
withkernel='linear'
had no function).I tried to find guidelines for this in the dev docs, but it either isn't there or I couldn't find it.
Just let me know if I should revert it to
DeprecationWarning
.Also wasn't sure if a note of this should be added to the docs, since they already describe which parameter affects which kernel.
Tests ran with Windows/Py3.8: