-
-
Notifications
You must be signed in to change notification settings - Fork 26k
ENH Add array_api compatibility to dcg_score
#29339
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
base: main
Are you sure you want to change the base?
Conversation
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.
Thanks for the PR @EdAbati
discount_cumsum = np.cumsum(discount) | ||
cumulative_gains = [ | ||
_tie_averaged_dcg(y_t, y_s, discount_cumsum) | ||
for y_t, y_s in zip(y_true, y_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.
Won't something like this work generically?
for y_t, y_s in zip(y_true, y_score) | |
for y_t, y_s in xp.stack((y_true, y_score), axis=1) |
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 seems to make array_api_strict
complain if y_true
and y_score
don't have the same type (like in this test case): TypeError: array_api_strict.int64 and array_api_strict.float64 cannot be type promoted together
(array_api_strict
complained a lot while working on this 😅)
Reference Issues/PRs
Towards #26024
What does this implement/fix? Explain your changes.
It makes the
dcg_score
implementation compatible and tested with the Array API.Any other comments?
TODOs
zip
doesn't seem to work witharray_api_strict
need to investigate (or find an alternative)_cumulative_sum
toutils
(and change name to_cumulative_sum1d
because does always_ravel
)float.64
withmps
Very happy to hear your feedback on how to improve this. (Mostly regarding the issue with Array API strict) I'm also a bit unsure if we have to split always numpy vs array-api in this case
cc @ogrisel @OmarManzoor