-
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
Added mathematical function jax.numpy.imag #18982
Conversation
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.
Hey @moezAhmad, requested some changes for you to look into. Do request a re-review once done. Thanks! :)
on_device=on_device, | ||
rtol=1e-2, | ||
atol=1e-2, | ||
input=x[0], |
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.
The input
here should instead be val
rtol=1e-2, | ||
atol=1e-2, |
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 the tests not passing with a low tolerance value? Somewhere around 1e-4
? 1e-2
seems to be too high of a tolerance value
# imag | ||
|
||
|
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.
Pretty sure these are lint/formatting issues. Can you kindly resolve these? :) We recommend using pre-commit
to automatically format your PRs every time you commit. Thanks
@hmahmood24 Thank you for reviewing my pr. I have implemented changes as you requested. |
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.
Everything seems good to merge now. Thanks @moezAhmad! 🚀
close #18981