Skip to content

[WIP] gamma=auto in SVC #8361#8535

Closed
neokt wants to merge 1 commit intoscikit-learn:masterfrom
neokt:svc_gamma
Closed

[WIP] gamma=auto in SVC #8361#8535
neokt wants to merge 1 commit intoscikit-learn:masterfrom
neokt:svc_gamma

Conversation

@neokt
Copy link
Copy Markdown

@neokt neokt commented Mar 4, 2017

Reference Issue

Addresses #8361

What does this implement/fix? Explain your changes.

Deprecates the default SVC gamma parameter value of "auto", which is calculated as 1 / n_features, and introduces "scale", which is calculated as 1 / (n_features * X.std()).

Any other comments?

Could not run nosetests due to problems with Conda environent. There are potentially other occurrences of SVC() that need to be updated to SVC(gamma="scale") to avoid Deprecation Warnings associated with SVC(gamma = "auto"). Submitting pull request to locate errors.

X, y = [[0.0], [1.0]], [0, 1]

msg = ("The default gamma parameter value 'auto', calculated as 1 / n_features,"
" is depreciated in version 0.19 and will be replaced by 'scale',"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we use "deprecated" not "depreciated"


clf = svm.SVC(gamma='scale').fit(X, y)
assert_equal(clf._gamma, 2.0)

No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there should be a newline at the end fo the file.

X, y = [[0.0], [1.0]], [0, 1]

clf = svm.SVC(gamma='scale').fit(X, y)
assert_equal(clf._gamma, 2.0)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please check for more than one X


assert_warns_message(DeprecationWarning,
msg,
svm.SVC(gamma='auto').fit, X, y)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But this means that a user can't intentionally pass 'auto' without receiving a warning, which isn't great. We could solve this by making the default actually 'auto_deprecated' which behaves like 'auto' with a warning. Using 'auto' explicitly would be same without warning.

random_state=rng)

for base_estimator in [DecisionTreeClassifier(), SVC()]:
for base_estimator in [DecisionTreeClassifier(), SVC(gamma="scale")]:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can use this PR to also set a random_state, to reduce diff of #8563.

It will anyway have a merge conflict for all these lines in that PR...

@jnothman Ok with you?

@gxyd
Copy link
Copy Markdown
Contributor

gxyd commented Dec 15, 2017

@neokt are you working on completing this? I just learnt about SVM's so, I can try to get this complete on top of your commits, if you don't plan on working on this.

@amueller amueller mentioned this pull request Dec 15, 2017
@amueller
Copy link
Copy Markdown
Member

@gxyd I think you can go ahead and work on this, since we haven't heard from @neokt in a while.

@gxyd
Copy link
Copy Markdown
Contributor

gxyd commented Dec 15, 2017

Thanks for the heads up. I'll try complete it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants