Skip to content

nanarg* broken #39

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
dcherian opened this issue Sep 9, 2021 · 2 comments
Closed

nanarg* broken #39

dcherian opened this issue Sep 9, 2021 · 2 comments

Comments

@dcherian
Copy link
Collaborator

dcherian commented Sep 9, 2021

from numpy_groupies.aggregate_numpy import aggregate

array = np.array([1, 1, 2, 1, 1, np.nan, 6, 1])
labels = np.array([0, 0, 0, 0, 0, 0, 0, 0])

func = "nanargmax"
actual = aggregate(labels, array, func=func).astype(int)
expected = getattr(np, func)(array)
np.testing.assert_equal(actual, expected)

fails with

AssertionError: 
Arrays are not equal

Mismatched elements: 1 / 1 (100%)
Max absolute difference: 1
Max relative difference: 0.16666667
 x: array([5])
 y: array(6)

I think the issue is that you can't use nansqueeze with nanarg* reductions:

if _nansqueeze:
good = ~np.isnan(a)
a = a[good]
group_idx = group_idx[good]

Or you could create a vector of indexes, apply the nan mask, and then index that vector with the current nanarg* output.

@ml31415
Copy link
Owner

ml31415 commented Oct 5, 2021

Hi Deepak! Thanks for your valuable input. So far I can confirm all your findings, and I'm a bit ashamed, that our test suite obviously has some gaps. PRs on that stuff would be highly welcome, not sure if I get to fix those errors in a timely manner.

ml31415 added a commit that referenced this issue Jul 12, 2022
@ml31415 ml31415 closed this as completed Jul 12, 2022
@ml31415
Copy link
Owner

ml31415 commented Jul 13, 2022

@dcherian please have a look if these fixes were sufficient for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants