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.

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

Regression Gamma Loss Gradient calculation is incorrect with weight #4174

Closed
katrat123 opened this issue Apr 13, 2021 · 9 comments · Fixed by #4283
Closed

Regression Gamma Loss Gradient calculation is incorrect with weight #4174

katrat123 opened this issue Apr 13, 2021 · 9 comments · Fixed by #4283
Labels
bug

Comments

@katrat123
Copy link

Description

In src/objective/regression_objective.hpp, line 698 the weight term is only being applied to a single term, rather than the entire expression. As a result, the weighted calculation of gamma objective is incorrect :
gradients[i] = static_cast<score_t>(1.0 - label_[i] / std::exp(score[i]) * weights_[i]);

Reproducible example

Lightgbm simplified code (incorrect) : (1.0 - y / exp(p) * w)
Xgboost simplified code (correct) : (1 - y / exp(p)) * w
Lightgbm simplifed code (correct) : (1-y/exp(p))*w
Tweedie, rho = 2 (gamma) simplified code : ((-y * exp(-1 * p) + exp(0 * p)) * w === (1 – y / exp(p)) * w
@jameslamb
Copy link
Collaborator

Thanks for opening an issue! Are you interested in contributing a fix?

@shiyu1994
Copy link
Collaborator

@katrat123 Yes, that's a mistake. Thanks for reporting that!

@katrat123
Copy link
Author

katrat123 commented Apr 15, 2021 via email

@jameslamb
Copy link
Collaborator

jameslamb commented Apr 15, 2021 •
edited
Loading

Yes please! The process is:

  1. Fork this repository
  2. create a branch on your fork
  3. push changes to that branch
  4. open a pull request at https://github.com/microsoft/LightGBM/pulls. End the title of the pull request with "(fixes Regression Gamma Loss Gradient calculation is incorrect with weight #4174)"

Let me know if you run into any issues! Thanks again for helping to improve LightGBM 🙌

@StrikerRUS StrikerRUS added the bug label Apr 17, 2021
@StrikerRUS
Copy link
Collaborator

Gently ping @katrat123

@katrat123
Copy link
Author

katrat123 commented Apr 30, 2021 via email

@mayer79
Copy link
Contributor

mayer79 commented May 13, 2021 •
edited
Loading

@StrikerRUS : Weighted Gamma regression is a quite typical model in insurance claim modelling. The response is the claim cost divided by the number of claims, while the weight is the number of claims. I think it would be good to fix this for the next release.
@katrat123 : If you do not have started yet and are short of time, I can offer you to do that pull request!

@katrat123
Copy link
Author

katrat123 commented May 13, 2021 via email

@mayer79 mayer79 mentioned this issue May 13, 2021
Merged
@jameslamb jameslamb mentioned this issue May 20, 2021
Closed
21 tasks
StrikerRUS pushed a commit that referenced this issue May 21, 2021
@mayer79
fix calculation of weighted gamma loss (fixes #4174) (#4283)
4b1b412 
* fixed weighted gamma obj

* added unit tests

* fixing linter errors

* another linter

* set seed

* fix linter (integer seed)
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix calculation of weighted gamma loss (fixes #4174)
5 participants
@jameslamb @shiyu1994 @mayer79 @StrikerRUS @katrat123