-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
Hypergeometric test and function redo #21922
Conversation
Thanks for contributing to Ivy! 😊👏 |
@to_ivy_arrays_and_back | ||
@from_zero_dim_arrays_to_scalar | ||
def hypergeometric(ngood, nbad, nsample, size=None): | ||
u = ivy.empty(size, dtype=int) |
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.
size
argument is default None which is a required argument for empty
which will always throw an error. As seen in the test logs here.
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.
Whenever I reproduce any of the failures I get an error that states that the test passed
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.
I tested the test cases locally and they don't fail like they do in the test suite. I even compared it to the numpy.random.hypergeometric module and they produce the same results (having in mind that the numbers are different due to the randomness)
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.
yea makes sense. The issue is a flaw in the logic. If you test call this function and don't specify a value for size
then this would break because None
is not a valid argument for size
in ivy.empty
.
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.
I dont get that issue in the new commits anymore
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.
Are all tests passing for you locally?
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.
locally I dont get any errors, yes
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.
I just want to mention, I have this many commits because my local tests only run for numpy and don't check the other backends.
Close #21387