[MRG] Change loss names for LinearSVC and LinearSVR by fabianp · Pull Request #3933 · scikit-learn/scikit-learn

fabianp · 2014-12-04T09:59:48Z

the names are now consistent across methods and with the SGD module

In LinearSVC:
'l1' -> 'hinge'
'l2' -> 'squared_hinge'
In LinearSVR:
'l1' -> 'epsilon_insensitive'
'l2' -> 'squared_epsilon_insensitive'

fabianp · 2014-12-04T10:02:00Z

The implementation is backwards compatible, so that the user can still use loss={'l1', 'l2'} in LinearSVC

mblondel · 2014-12-04T11:19:51Z

Can you add a test for loss="squared_epsilon_insensitive"?

larsmans · 2014-12-04T12:51:48Z

sklearn/svm/base.py

I'm having a hard parsing the meaning of this... care to insert a comment explaining the structure?

amueller · 2014-12-04T17:46:10Z

I guess we should deprecate the old names then? Is that consistent with SGDClassifier?

larsmans · 2014-12-04T20:25:28Z

Or we don't deprecate, and we leave them in-place forever as backwards-compatible synonyms. They're not in the way, are they?

amueller · 2014-12-04T20:41:56Z

Fine with me, but then they should be documented, right?
If someone sees it used in the code, then looks at the documentation and it is not documented as a legal option, that seems weird.

fabianp · 2014-12-05T05:45:18Z

I agree it should be documented, I'll take care of it (and the failing tests).

fabianp · 2014-12-10T10:12:17Z

Fixed the failing test (mostly due to error in doctests). Ready to merge I would say

larsmans · 2014-12-15T17:13:55Z

+1 for merge. I've already cherry-picked the last commit into master.

amueller · 2014-12-15T17:19:03Z

sklearn/svm/classes.py

we that's why we shouldn't do it in the init method ;) actually the common tests should fail if you do it in the __init__. Do they not? The deprecation warning should give the version (0.17?) where it will be removed.

Ohh, missed this one. I too think we should do this in fit and not actually store the translated loss name. Just make sure l1 and hinge cause the same behavior.

fabianp · 2014-12-16T08:25:15Z

Thanks for your feedback @amueller and @larsmans! I fixed it so that the parameters are converted in the .fit() but the modification is never stored.

amueller · 2014-12-16T16:43:34Z

👍

larsmans · 2014-12-18T15:37:38Z

Ehm... it still says the old names are going to disappear in 0.17. Can we really not keep them indefinitely? Or remove in version 1.0 or something?

fabianp · 2014-12-18T16:50:05Z

I really don't mind in what version to deprecate. 1.0 is fine for me.

fabianp · 2015-02-06T13:43:37Z

Changed the removal version to 1.0

ogrisel · 2015-02-06T18:13:02Z

Fine with me. This PR needs a rebase though.

the names are now consistent across methods In LinearSVC: 'l1' -> 'hinge' 'l2' -> 'squared_hinge' In LinearSVR: 'l1' -> 'epsilon_insensitive' 'l2' -> 'squared_epsilon_insensitive'

fabianp · 2015-02-09T10:10:08Z

Rebase done

MAINT Change loss names for LinearSVC and LinearSVR

larsmans · 2015-02-10T16:49:18Z

Merged.

fabianp mentioned this pull request Dec 4, 2014

[MRG+1] Implementation of LinearSVR using liblinear code #3877

Closed

larsmans reviewed Dec 4, 2014
View reviewed changes

sklearn/svm/base.py

Copy link
Copy Markdown

Member

larsmans Dec 4, 2014

Choose a reason for hiding this comment

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

I'm having a hard parsing the meaning of this... care to insert a comment explaining the structure?

fabianp force-pushed the loss_liblinear branch from 3a5b451 to 3f25c73 Compare December 9, 2014 15:40

fabianp changed the title ~~Change loss names for LinearSVC and LinearSVR()~~ [MRG] Change loss names for LinearSVC and LinearSVR() Dec 10, 2014

larsmans changed the title ~~[MRG] Change loss names for LinearSVC and LinearSVR()~~ [MRG+1] Change loss names for LinearSVC and LinearSVR Dec 15, 2014

amueller reviewed Dec 15, 2014
View reviewed changes

larsmans changed the title ~~[MRG+1] Change loss names for LinearSVC and LinearSVR~~ [MRG] Change loss names for LinearSVC and LinearSVR Dec 15, 2014

fabianp force-pushed the loss_liblinear branch from f9852a5 to d49a935 Compare December 16, 2014 07:51

fabianp force-pushed the loss_liblinear branch from bc76ecf to b56f7e5 Compare February 9, 2015 10:07

Change loss names for LinearSVC and LinearSVR()

dbc5d70

the names are now consistent across methods In LinearSVC: 'l1' -> 'hinge' 'l2' -> 'squared_hinge' In LinearSVR: 'l1' -> 'epsilon_insensitive' 'l2' -> 'squared_epsilon_insensitive'

fabianp force-pushed the loss_liblinear branch from b56f7e5 to dbc5d70 Compare February 9, 2015 10:09

larsmans added a commit that referenced this pull request Feb 10, 2015

Merge pull request #3933 from fabianp/loss_liblinear

73e5cf5

MAINT Change loss names for LinearSVC and LinearSVR

larsmans merged commit 73e5cf5 into scikit-learn:master Feb 10, 2015

raghavrv mentioned this pull request Feb 17, 2015

[MRG+1] Fix broken L1 deprecations and svm scale C example. #4261

Merged

4 tasks

Uh oh!

Conversation

fabianp commented Dec 4, 2014

Uh oh!

fabianp commented Dec 4, 2014

Uh oh!

mblondel commented Dec 4, 2014

Uh oh!

larsmans Dec 4, 2014

Choose a reason for hiding this comment

Uh oh!

amueller commented Dec 4, 2014

Uh oh!

larsmans commented Dec 4, 2014

Uh oh!

amueller commented Dec 4, 2014

Uh oh!

fabianp commented Dec 5, 2014

Uh oh!

fabianp commented Dec 10, 2014

Uh oh!

larsmans commented Dec 15, 2014

Uh oh!

amueller Dec 15, 2014

Choose a reason for hiding this comment

Uh oh!

larsmans Dec 15, 2014

Choose a reason for hiding this comment

Uh oh!

fabianp commented Dec 16, 2014

Uh oh!

amueller commented Dec 16, 2014

Uh oh!

larsmans commented Dec 18, 2014

Uh oh!

fabianp commented Dec 18, 2014

Uh oh!

fabianp commented Feb 6, 2015

Uh oh!

ogrisel commented Feb 6, 2015

Uh oh!

fabianp commented Feb 9, 2015

Uh oh!

larsmans commented Feb 10, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants