Skip to content

[WIP] loss function name consistency #3556

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

[WIP] loss function name consistency #3556

wants to merge 2 commits into from

Conversation

dsullivan7
Copy link
Contributor

This is a checkpoint to solve #3481

jnothman
jnothman reviewed Aug 13, 2014
View reviewed changes
sklearn/ensemble/gradient_boosting.py
loss function to be optimized. 'ls' refers to least squares
regression. 'lad' (least absolute deviation) is a highly robust
loss function solely based on order information of the input
loss : {'ls', 'lad', 'absolute', huber', 'quantile'},
Copy link
Member

Choose a reason for hiding this comment

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

huber' -> 'huber'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you got it

Copy link
Member

Choose a reason for hiding this comment

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

It would mention only absolute and not lad in the docstring. Same for squared.

@amueller amueller added Easy Well-defined and straightforward way to resolve Stalled help wanted labels Sep 27, 2018
@PrasadiApsara
Copy link

@amueller Hi can I work on this? I am looking for a first issue to start working on :)

@dsullivan7
Copy link
Contributor Author

@PrasadiApsara No problem from me if you want to take over the branch.

@PrasadiApsara
Copy link

@dsullivan7 Thanks, I will get the branch.

@PrasadiApsara
Copy link

take

@PrasadiApsara
Copy link

@dsullivan7 I went through the PR and the discussion. So based on that if I summarize what is there to be done,

  1. Add absolute, squared, squared_hinge and modified_huber to gradient boosting
  2. Rename the squared_loss in SGD to squared
  3. SVM is already fixed it seems

Let me know whether I have got it correctly.

@lorentzenchr
Copy link
Member

@PrasadiApsara Thanks for bringing this one back on the table. Note that #18248 supersedes #3481, because it aims for a broader consistency of loss names over all modules, but needs discussion first.

Maybe we should close this PR. Would that be OK, @dsullivan7?

@dsullivan7
Copy link
Contributor Author

@lorentzenchr Yes, that's fine!

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

@PrasadiApsara PrasadiApsara

Labels
Easy Well-defined and straightforward way to resolve module:ensemble module:svm Stalled
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dsullivan7 @PrasadiApsara @lorentzenchr @jnothman @mblondel @amueller