8000 FEA Add d2_log_loss_score by OmarManzoor · Pull Request #28351 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 16 commits into from
May 2, 2024
Merged

Conversation

OmarManzoor
Copy link
Contributor

Reference Issues/PRs

Fixes: #20943

What does this implement/fix? Explain your changes.

  • Adds d2_log_loss_score

Any other comments?

Copy link
github-actions bot commented Feb 2, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 1dd08df. Link to the linter CI: here

@OmarManzoor
Copy link
Contributor Author

@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.

Copy link
Member
@lorentzenchr lorentzenchr left a 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.

@OmarManzoor OmarManzoor marked this pull request as ready for review February 12, 2024 09:45
Copy link
Member
@lorentzenchr lorentzenchr left a comment

Choose a reason for hiding this comment

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

Close to finish.

@OmarManzoor
Copy link
Contributor Author

< 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?

@lorentzenchr lorentzenchr added this to the 1.5 milestone Apr 30, 2024
Copy link
Member
@jeremiedbb jeremiedbb left a 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.

Comment on lines 3361 to 3364
if sample_weight is not None:
weights = np.asarray(sample_weight)
else:
weights = np.ones(shape=len(y_true), dtype=np.int64)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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)

Comment on lines 2833 to 2840
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::
Copy link
Member

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.

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,)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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,)

Copy link
Member
@jeremiedbb jeremiedbb left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @OmarManzoor

@jeremiedbb jeremiedbb enabled auto-merge (squash) May 2, 2024 13:50
@@ -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**
Copy link
Member

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.

Copy link
Member

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 😄

@jeremiedbb jeremiedbb merged commit 8d89e8d into scikit-learn:main May 2, 2024
30 checks passed
@OmarManzoor OmarManzoor deleted the d2_log_loss branch May 2, 2024 14:23
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.

Add more D2 scores
3 participants
0