-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
FEA Add d2_log_loss_score #28351
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
FEA Add d2_log_loss_score #28351
Conversation
@lorentzenchr Could you kindly have a look to see if this formulation looks okay? If this looks correct then we can add the general case which handles more than one class and also all the corresponding things required like the addition of documentation, more tests etc. step by step. |
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.
This goes in the right direction.
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.
Close to finish.
< 6293 a class="user-mention notranslate" data-hovercard-type="user" data-hovercard-url="/users/lorentzenchr/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/lorentzenchr">@lorentzenchr Would it make sense to include this in 1.5? |
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.
Looks good, just a few minor comments.
sklearn/metrics/_classification.py
Outdated
if sample_weight is not None: | ||
weights = np.asarray(sample_weight) | ||
else: | ||
weights = np.ones(shape=len(y_true), dtype=np.int64) |
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.
if sample_weight is not None: | |
weights = np.asarray(sample_weight) | |
else: | |
weights = np.ones(shape=len(y_true), dtype=np.int64) | |
weights = _check_sample_weight(sample_weight, y_true) |
doc/modules/model_evaluation.rst
Outdated
The :func:`d2_log_loss_score` function implements the special case | ||
of D² with the log loss, see :ref:`log_loss`, i.e.: | ||
|
||
.. math:: | ||
|
||
\text{dev}(y, \hat{y}) = \text{log_loss}(y, \hat{y}). | ||
|
||
Here are some usage examples of the :func:`d2_log_loss_score` function:: |
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.
Please mention that the y_null for the log loss is the per-class frequency, here or in the introdction paragraph above where the y_null for other losses are described.
sklearn/metrics/_classification.py
Outdated
y_true : array-like or label indicator matrix | ||
The actuals labels for the n_samples samples. | ||
|
||
y_pred : array-like of float, shape = (n_samples, n_classes) or (n_samples,) |
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.
y_pred : array-like of float, shape = (n_samples, n_classes) or (n_samples,) | |
y_pred : array-like of shape (n_samples, n_classes) or (n_samples,) |
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.
LGTM. Thanks @OmarManzoor
@@ -2826,6 +2826,51 @@ Here are some usage examples of the :func:`d2_absolute_error_score` function:: | |||
|
|||
|details-end| | |||
|
|||
|details-start| | |||
**D² log loss score** |
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.
This is under „regression metrics“ but it’s a classification metric. Wen can fix that in another PR as it involves a larger change of the user guide.
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.
Haha you spot it as well. I assumed I would just merge it discretly and open an issue right after 😄
Reference Issues/PRs
Fixes: #20943
What does this implement/fix? Explain your changes.
Any other comments?