-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC document about the divergent factor of the Binomial loss #25383
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
Conversation
I don't think we should change the description of the hyperparameter that is use to select the loss to optimize for when fitting the model. The original problem was about the content of the Furthermore, I think we might want store the real full loss value instead of the half-loss value inside that attribute. That should not be expensive to compute because it is not a per-sample operation but a per-epoch operation. WDYT @lorentzenchr and others? |
@@ -883,6 +883,7 @@ class GradientBoostingClassifier(ClassifierMixin, BaseGradientBoosting): | |||
loss : {'log_loss', 'deviance', 'exponential'}, default='log_loss' | |||
The loss function to be optimized. 'log_loss' refers to binomial and | |||
multinomial deviance, the same as used in logistic regression. | |||
It contains a factor x2 that have to be neglected to reflect the actual loss. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is only true for the log_loss
and deviance
(which is the same loss).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way to check is to look at how the loss functions are defined in sklearn/_loss/loss.py
: the loss function classes that have "Half" in their name only compute half of the usual value (for performance reasons when computing the per-sample gradients).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually not for the GradientBoosting. I think that we still import from _gb_losses.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is as @glemaitre says. The change to the loss function module was blocked by a deprecation period for the loss_
attribute of GradientBoostingRegressor
. This is now over (1.3).
This would be fine. With version 1.3., |
@ogrisel @glemaitre @lorentzenchr what's the status here now? it's been stale since January. |
Right now, I‘d like to first finish #25964. |
Looks like the solution to the issue is quite different than this proposal. Therefore closing and happy to have a fresh PR with suggested changes here. |
Reference Issues/PRs
Fixes #25206What does this implement/fix? Explain your changes.
I did a documentation change where the deviance(=loss) given by the train_score_ is double the actual log_loss which gave some non understandable results when using the train_score_ functions for the training data and a classic log_normal function for the test data.
The fix i proposed is on the level of the documentary of the parameters of the loss of the GradientBoostingClassifier where i mentionned that loss(=deviance) might have a factor of two and that if we want the actual we need to take only half the loss.
Any other comments?