8000 FIX #6076. Bug in QuantileLossFunction by devashishd12 · Pull Request #6133 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

FIX #6076. Bug in QuantileLossFunction #6133

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 1 commit into from
Closed

FIX #6076. Bug in QuantileLossFunction #6133

wants to merge 1 commit into from

Conversation

devashishd12
Copy link
Contributor

Fixes #6076. Flipped diff sign for cases of ~mask.

@devashishd12
FIX #6076. Bug in QuantileLossFunction
caa34cf 
Flipped `diff` sign for cases of `~mask`.
@devashishd12
Copy link
Contributor Author

Could someone please check this?

@devashishd12
Copy link
Contributor Author

@glouppe could you please take a look at this? Sorry for bothering you but I just saw your name in the authors list for that code so I thought you could guide me here.....

@glouppe
Copy link
Contributor
glouppe commented Jan 13, 2016

Hi! Can you elaborate as to why this should be negated?

@devashishd12
Copy link
Contributor Author

I just tried following the formula for quantile loss given here: https://github.com/JohnLangford/vowpal_wabbit/wiki/Loss-functions. But I'm still not sure to tell you the truth.... Please tell me if I've horribly misunderstood the formula and/or the code :/

@ahyoussef
Copy link

Adding @pprett since he wrote most of the code.

There is indeed a sign error. Here are some simple ways to convince yourselves:

  • The loss function needs to be positive. As you can see, the current implementation will give a negative result if y < pred.

  • For alpha=0.5, the quantile loss should be proportional to the least absolute deviation.

  • Finally You can look at formula (2) of this paper: http://avesbiodiv.mncn.csic.es/estadistica/curso2011/qr4.pdf

To summarize you need to make exactly two changes:

  1. Replace mask = y>pred, by mask = y >= pred

  2. Replace + (1.0 - alpha) diff[~mask] by - (1.0 - alpha) diff[~mask]

  3. The negative gradient seems correct.

@amueller
Copy link
Member

@ahyoussef could you please open an issue? It's not possible to reopen this PR.

@amueller
Copy link
Member

@ahyoussef it would also be helpful if you could provide a self-contained example to exemplify the error.

@ahyoussef ahyoussef mentioned this pull request Dec 19, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@devashishd12 @glouppe @ahyoussef @amueller
0