Skip to content

array API support for mean_gamma_deviance #29239

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 9 commits into from
Jun 13, 2024

Conversation

EmilyXinyi
Copy link
Contributor

Reference Issues/PRs

towards #26024

What does this implement/fix? Explain your changes.

add array API support for mean_gamma_deviance

Any other comments?

mean_gamma_deviance is a special case of mean_tweedie_deviance where power=2 and both y_pred and y_true must be strictly positive. For this reason I have added the test case check_array_api_regression_metric_gamma (because the y_true in check_array_api_regression_metric contains 0 and I didn't want to change a test that is so widely used). I am not sure if this is the best way to approach this, so if there are any suggestions on how to do this better I would love to know. Thanks!!

@EmilyXinyi EmilyXinyi marked this pull request as ready for review June 11, 2024 16:09
Copy link

github-actions bot commented Jun 11, 2024

✔️ Linting Passed

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

Generated for commit: 57b0434. Link to the linter CI: here

Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I launched the CUDA tests here:

EDIT: they pass.

Assuming they pass, LGTM. Just a suggestion to simplify the tests below:

@ogrisel ogrisel added Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one! labels Jun 12, 2024
@EmilyXinyi EmilyXinyi changed the title Array api mean gamma deviance array API support for mean_gamma_deviance Jun 13, 2024
Copy link
Member

@betatim betatim left a comment

Choose a reason for hiding this comment

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

LGTM.

I resolved the conflict and enabled auto-merge

@betatim betatim enabled auto-merge (squash) June 13, 2024 13:57
@betatim betatim merged commit f10c171 into scikit-learn:main Jun 13, 2024
28 checks passed
@jeremiedbb jeremiedbb mentioned this pull request Jul 2, 2024
11 tasks
@EmilyXinyi EmilyXinyi deleted the array_API_mean_gamma_deviance branch August 12, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Array API module:metrics Quick Review For PRs that are quick to review Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants