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

Removing the wrong gamma-deviance metric #6728

Closed
wants to merge 1 commit into from

Conversation

Roffild
Copy link
Contributor

@Roffild Roffild commented Feb 24, 2021

Corrected version, but without checking the condition label>0 && pred>0:

struct EvalGammaDeviance {
  const char *Name() const {
    return "gamma-deviance";
  }

  XGBOOST_DEVICE bst_float EvalRow(bst_float label, bst_float pred) const {
    const bst_float eps = 1e-16f;
    return 2 * (std::log(pred/(label+eps)) + label/(pred+eps) - 1);
  }
  static bst_float GetFinal(bst_float esum, bst_float wsum) {
    return wsum == 0 ? esum : esum / wsum;
  }
};

@Roffild Roffild mentioned this pull request Feb 24, 2021
Copy link
Contributor Author

@Roffild Roffild left a comment

Choose a reason for hiding this comment

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

Approved

Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

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

Good catch! But could you please replace it with the correct one instead? The metrics defined in xgboost have many use like distributed setting, language bindings other than Python. Also 3 party implementation cannot be saved.

@Roffild
Copy link
Contributor Author

Roffild commented Feb 25, 2021

This is part of Tweedie. And everything about Tweedie needs to be redone. I don't understand what formulas were used for Tweedie in XGBoost.

I am guided by these sources:
https://en.wikipedia.org/wiki/Tweedie_distribution#The_Tweedie_deviance

sklearn/_loss/glm_distribution.py

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.

2 participants