ENH Add array api support for precision, recall and fbeta_score - #30395
Conversation
virchan
left a comment
virchan
left a comment
There was a problem hiding this comment.
LGTM! Just one minor suggestion:
|
LGTM, just one question about removing the assert statement. |
|
@ogrisel Do you think we can merge this PR? |
| denom = beta2 * xp.asarray( | ||
| true_sum, dtype=max_float_type, device=device_ | ||
| ) + xp.asarray(pred_sum, dtype=max_float_type, device=device_) |
There was a problem hiding this comment.
shouldn't true_sum and pred_sum already be on the device? I don't like that a simple multiplication and sum has become such a long piece of code.
There was a problem hiding this comment.
Array api strict is the only one which causes such issues. They require that the arrays be of the same type. Over here since beta2 is a float it doesn't accept ints and raises the following error:
TypeError: array_api_strict.float64 and array_api_strict.int64 cannot be type promoted together
Anyways I made some changes to only cast it to the correct dtype.
There was a problem hiding this comment.
Note: I verified that the CUDA tests work fine on a kaggle kernel
| check_array_api_multilabel_classification_metric, | ||
| ], | ||
| fbeta_score: [ | ||
| check_array_api_multiclass_classification_metric, |
There was a problem hiding this comment.
@OmarManzoor naive question - why not include check_array_api_binary_classification_metric here?
There was a problem hiding this comment.
If it works for the binary case I think we can include it.
There was a problem hiding this comment.
Ah thanks! I was just wondering if it was left out for a specific reason!
5 participants
Reference Issues/PRs
Towards #26024
What does this implement/fix? Explain your changes.
Any other comments?
CC: @ogrisel @adrinjalali