| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Adds an Array API-compatible top_k wrapper to the array_api_compat.torch namespace, mapping the standardized signature to torch.topk.
Changes:
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sorry, something went wrong.
|
So torch.topk needs only a small tweak to make it spec-compliant, which is exactly the remit of array-api-compat. Next is what to do about older numpies and cupy, which do not have native implementations. |
Sorry, something went wrong.
|
Older numpy/cupy: I can see the case either way. Pinging @betatim for scikit-learn needs, and @MaanasArora in case he has good ideas after working on the numpy implementation. |
Sorry, something went wrong.
|
What are the options? My two thoughts/inputs:
|
Sorry, something went wrong.
As far as I understand, for NumPy at least (and the link you referenced) we are using argpartition instead of argsorts. This is the same operation that will be used in the native implementation for NumPy 2.6. The missing feature in numpy <= 2.5 is descending argpartitions, which means that the largest=True case requires special Python-level handling to flip the kth and ensure nans are handled correctly (which was present in the older NumPy implementation that data-apis/array-api-strict#215 seems to use). Given that the most expensive underlying operation is the same, I wouldn't expect an outsized performance difference for a compatibility layer. Whether the complexity of manual nan and kth handling is worth supporting older NumPy is a different question. Given the performance should be in a similar order, I'd lean towards supporting if it is very useful for client libraries. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Add basic top_k wrappers, per data-apis/array-api#722
-tests tracker: data-apis/array-api-tests#438
Supersedes and closes gh-158
To test locally: with this PR and data-apis/array-api-tests#438, use