8000 DOC - Specify penalty in ``GammaRegressor`` documentation by Badr-MOUFAD · Pull Request #24789 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

DOC - Specify penalty in GammaRegressor documentation #24789

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

Merged
merged 10 commits into from
Nov 13, 2022

Conversation

Badr-MOUFAD
Copy link
Contributor

Reference Issues/PRs

Not linked to any exiting issue or PR.

What does this implement/fix? Explain your changes.

This fixes GammaRegressor documentation. In particular:

  1. adds the optimized objective
  2. specify penalty type
  3. doc formatting

Any other comments?

No.


This regressor uses the 'log' link function.
Its optimization objective is

.. math::
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Badr-MOUFAD Badr-MOUFAD changed the title Doc - Add objective function and specify penalty in GammaRegressor documentation DOC - Add objective function and specify penalty in GammaRegressor documentation Nov 1, 2022
Comment on lines 616 to 619
Its optimization objective is::

sum(Xw_i + y_i * exp(-Xw_i) - log(y_i) - 1)
+ (1/2) * alpha ||w||_2^2
Copy link
Member

Choose a reason for hiding this comment

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

I am still not sure that we want to have the formulation here.

@lorentzenchr What is your feeling about it? If we start to introduce the objective function here, then we probably want to do the same for the other GLM.

Copy link
Member

Choose a reason for hiding this comment

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

I personally like the objective as part of the docstring, but we often only have them in the user guide, examples: LogisticRegression, PoissonRegressor, GammaRegressor.

In case we include them in the docstring, having them as $\LaTeX$ would be much more readable. Therefore, having it in the Notes section would be numpydoc style compliant, https://numpydoc.readthedocs.io/en/latest/format.html#notes.

Copy link
Member

Choose a reason for hiding this comment

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

Cons: It could be considered as implementation detail. It bloats the docstring. We would have it in 2 places, user guide and docstring.

Pros: Without objective, one does not know what the model does. Only the exact objective enables to compare with other libraries.

@Badr-MOUFAD Badr-MOUFAD changed the title DOC - Add objective function and specify penalty in GammaRegressor documentation DOC - Specify penalty in GammaRegressor documentation Nov 12, 2022
@Badr-MOUFAD
Copy link
Contributor Author
Badr-MOUFAD commented Nov 12, 2022

I agree with you @glemaitre on #24789 (comment), better not to add the objective function expression in this place.

I removed it and kept the details about the penalty term and the doc formatting.

@glemaitre glemaitre merged commit 49aae1c into scikit-learn:main Nov 13, 2022
@glemaitre
Copy link
Member

Thanks @Badr-MOUFAD LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0