-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
cupy.sign(NaN) == 0, unlike numpy #8327
Comments
It looks like the kernel is defined as numpy uses the expression Some brief testing in godbolt shows that with nvcc 11.7, the numpy expression compiles into the same number or fewer of PTX instructions, without any branches. (Even fewer with floating point if the |
@prutschman-iv Thanks for reporting this one and also checking the performance impact! Indeed it's better to fix this one. Would you be interested in submitting a pull request to address this bug? |
I'll attempt to prepare a pull request with appropriate tests. For the tests, it looks like one approach would be adding something like this to test_misc.py. Does that seem right?
(Also, I just realized that |
I must be misunderstanding some aspect of the test suite. I tried adding the (I'm currently using the v12 branch because I couldn't get the main branch to build due to a problem with dlpack) |
Thanks @prutschman-iv! The test looks legitimate to me. To run the test please use |
I was successful in running the test suite for v12, thank you. From there I also observed a divergence between cupy and numpy for complex numbers, so I've fixed that as well. From the discussion linking to this one it sounds like there is maybe some controversy about the "correct" behavior? But my fix causes the behavior to match numpy's behavior as of 1.26. The efficiency story isn't as rosy for the complex numbers case; it appears that the existing code was complicated enough to cause branches rather than predicated execution already and the extra test for nan does not improve the situation. Unfortunately, I'm still having trouble building on main, even on a completely fresh clone into an new directory. For some reason the file |
@prutschman-iv feel free to file a draft PR so that one of us can look at it and help you verify the issues :) |
On Windows, symlink support needs to be enabled manually: |
I'm not trying to pester, but I wanted to point out that I've removed the draft status from my PR, just in case that notification isn't automatic. |
Description
numpy.sign(numpy.nan) returns NaN, but cupy.sign(cupy.nan) returns 0.
I expected either the behavior to be the same, or the difference to be documented. (Edit: I see now that the different behavior is documented, but not explicitly contrasted with numpy in the documentation. Apologies if this is better suited as a documentation enhancement, although I would prefer the matching behavior.)
To Reproduce
Installation
Wheel (
pip install cupy-***
)Environment
Additional Information
No response
The text was updated successfully, but these errors were encountered: