-
-
Notifications
You must be signed in to change notification settings - Fork 10.8k
EHN: add numpy.topk #19117
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?
EHN: add numpy.topk #19117
Conversation
numpy/core/fromnumeric.py
Outdated
if largest: | ||
index_array = np.argpartition(-a, k-1, axis=axis, order=None) | ||
else: | ||
index_array = np.argpartition(a, k-1, axis=axis, order=None) |
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.
Negating like this won't work for unsigned types
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 your reply, I will test it for unsigned types again.
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.
Hi, I have fixed this bug with the follwing codes and updated test codes.
if largest:
index_array = np.argpartition(a, axis_size-k, axis=axis)
topk_indices = np.take(index_array, -np.arange(k)-1, axis=axis)
else:
index_array = np.argpartition(a, k-1, axis=axis)
topk_indices = np.take(index_array, np.arange(k), axis=axis)
Note that as a new bit of API, this should hit the mailing list. My impression is that in its current form The |
Are there any cases wherein reversal cannot be easily achieved with, e.g., the likes of |
Not that I think this is a great fit, but |
Following previous discussion at numpy#15128. I made a small change to the interface in the previous discussion by changing the `mode` keyword into a `largest` bool flag. This follows API such as from [torch.topk](https://pytorch.org/docs/stable/generated/torch.topk.html). Carrying from the previous discussion, a parameter might be useful is `sorted`. This is also implemented in `torch.topk`, and follows from previous work at numpy#19117. Co-authored-by: quarrying
Hi, find topk elements is widely used in machine learning and elsewhere, and some python frameworks have implemented it, e.g. TensorFlow (
tf.math.top_k
), PyTorch (torch.topk
).I try to implement topk using core numpy functions, and have checked its correctness using
torch.topk
. The following is the test code snippet: