8000 BUG: `np.unique` with `return_inverse` and `axis` specification yields a wrong shape · Issue #26738 · numpy/numpy · GitHub
[go: up one dir, main page]

Skip to content

BUG: np.unique with return_inverse and axis specification yields a wrong shape #26738

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

Closed
nabenabe0928 opened this issue Jun 18, 2024 · 7 comments

Comments

@nabenabe0928
Copy link

Describe the issue:

In NumPy v2.0.0, when I specify return_inverse=True and axis for a 2d array, np.unique will return a wrongly deep-nested array for the inverse.

Reproduce the code example:

import numpy as np


a = np.array([[3, 1], [1, 2], [3, 1]])
u, inv = np.unique(a, return_inverse=True)
a1 = u[inv]
assert np.all(a1 == a)  # True

u, inv = np.unique(a, return_inverse=True, axis=0)
a2 = u[inv.flatten()]
a3 = u[inv]
assert np.all(a2 == a)  # True
assert np.all(a3 == a)  # False

Error message:

No response

Python and NumPy Versions:

  • NumPy: 2.0.0
  • Python: 3.9.13 (main, Aug 25 2022, 23:26:10)
    [GCC 11.2.0]

Runtime Environment:

No response

Context for the issue:

No response

@seberg
Copy link
Member
seberg commented Jun 18, 2024

This was intentional: https://numpy.org/devdocs/release/2.0.0-notes.html#np-unique-return-inverse-shape-for-multi-dimensional-inputs

It is now compatible with np.take_along_axes.

But we may have underestimated the impact of this choice, and whether it is even the right one, since previously it was compatible with np.take.

The reason for this compatibility is that unlike argmin/argmax the unspecified axes are all part of the operation and not broadcast axes.

To clarify, let me invent a different operation that behaves similar to return_inverse:

def filter_by_sum(a, axis=0):
    axes_to_sum = list(range(0, a.ndim))
    axes_to_sum.remove(axis)
    use = a.sum(axis=axes_to_sum)
    return a.take(use > 1, axis=axis)

The sum operation there uses all but the specified axes.

That means that both choices make sense, since using take is a correct choice if there are no broadcast axes in the operation and take_along_axes is the right choice if there are only broadcast axes.

One option is just to undo this for unique but keep the take-along-axes behavior for the new ones. When the change was made, it sounded to me like the current behavior doesn't make sense. But it seems like it does because the other axes are not broadcast-axes for unique (and I guess never will be).

Unless we could even reconsider it for the unique_*?

CC @jakevdp, @rgommers.

@seberg
Copy link
Member
seberg commented Jun 18, 2024

Ah, one thing I missed and this does indeed improve, is that for axis=None returning this shape does simplify reconstructing the original array.
Although, interestingly if paired with np.take() not with np.take_along_axes().

@seberg
Copy link
Member
seberg commented Jun 18, 2024

@nabenabe0928 I forgot to mention that the work-around is to add a .reshape(-1) to the result, I think. And whatever happens, there is a small chance it could become a no-op, but it'll keep working on any version.

@nabenabe0928
Copy link
Author
nabenabe0928 commented Jun 19, 2024

This was intentional: https://numpy.org/devdocs/release/2.0.0-notes.html#np-unique-return-inverse-shape-for-multi-dimensional-inputs

@seberg
Thank you for the information, I completely had missed the context!

Tbh, I did not know np.take and np.take_along_axis either.

This breaking change was a bit troublesome for me because I was using np.unique(..., return_inverse=True) as a lexicographic sorting function and its inverse array.
With this change, the inverse function looked a bit unnatural when specifying an axis although I agree that the behavior for axis=None is now much more natural.

Note

I know that there is np.lexsort, but in my usecase, the shape of an array is (N, M) where both N and M can change arbitrarily and I wanted to make the array unique as well, so np.unique was syntactically cleaner and more efficient.

Anyways, given the context that the change is intended, I will simply use reshape or flatten to avoid the problem.

@jakevdp
Copy link
Contributor
jakevdp commented Jun 21, 2024

For what it's worth, this was the kind of issue I had in mind when I commented here on the original PR: #25553 (comment), but the NumPy team together decided that the breaking change was warranted: #25553 (comment)

I'm not sure whether there are any public notes from that discussion.

@seberg
Copy link
Member
seberg commented Jun 21, 2024

Right, the thing that I misunderstood, is that I had the impression that new behavior was chosen because the old behavior was very strange. But that seems wrong, I am not even sure why the new behavior is in any serious way better right now (besides for axis=None maybe and I am not sure how much it matters).

Dunno, its public, but of there are unlikely to be detailed enough notes.

@seberg
Copy link
Member
seberg commented Jul 13, 2024

Closing, this is reverted. Note that the shape did change in 2.0 for the first case you mentioned and will stay changed. The first example actually didn't work previously.

@seberg seberg closed this as completed Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants
0