Skip to content
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

[SPARK-20348] [ML] Support squared hinge loss (L2 loss) for LinearSVC #17645

Closed
wants to merge 1 commit into from

Conversation

hhbyyh
Copy link
Contributor

@hhbyyh hhbyyh commented Apr 16, 2017

What changes were proposed in this pull request?

While Hinge loss is the standard loss function for linear SVM, Squared hinge loss (a.k.a. L2 loss) is also popular in practice. L2-SVM is differentiable and imposes a bigger (quadratic vs. linear) loss for points which violate the margin. Some introduction can be found from http://mccormickml.com/2015/01/06/what-is-an-l2-svm/

Liblinear and scikit learn both offer squared hinge loss as the default loss function for linear SVM.

How was this patch tested?

strengthen existing unit test and add new unit test for comparison.

@SparkQA
Copy link

SparkQA commented Apr 16, 2017

Test build #75830 has finished for PR 17645 at commit 6541f69.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

* @group param
*/
@Since("2.3.0")
final val lossFunction: Param[String] = new Param(this, "lossFunction", "Specifies the loss " +
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to move this out to shared params, since it can be used by other algorithms as well. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure we can do it.
But I'm thinking maybe we should conduct an integrated refactor about the common optimization parameters some time in the future, either through shared params or other trait or abstract class.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, let leave as is and refactor in the future. One minor issue: What about renaming it to loss? I found the name of corresponding params in sklearn.svm.linearSVC is loss. Thanks.

@HyukjinKwon
Copy link
Member

ping @hhbyyh, where are we on this?

@hhbyyh
Copy link
Contributor Author

hhbyyh commented Jun 7, 2017

Hi @HyukjinKwon I think this is a feature we need, but currently we are still having some discussion about optimizer interface.

@HyukjinKwon
Copy link
Member

HyukjinKwon commented Jun 7, 2017

I took out this in the list. Though, shouldn't we maybe close this for now and reopen again when it's ready if it takes quite long? It'd be probably better than leaving this open without further updates for long time.

@HyukjinKwon
Copy link
Member

ping @hhbyyh. WDYT?

@hhbyyh
Copy link
Contributor Author

hhbyyh commented Jun 12, 2017

OK. I'll close it for now and try to merge it with #17862.
Thanks for the comment from @yanboliang

@hhbyyh hhbyyh closed this Jun 12, 2017
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