Minor fixes for NumPy 2.0 compatibility #746
Conversation
rtol of 1e-6 is sufficient given that the precision used internally is always float32
grlee77
added
improvement
| @@ -429,6 +429,7 @@ def chan_vese( | |||
| energies = [] | |||
| phivar = tol + 1 | |||
|
|
|||
| dt = cp.asarray(dt, dtype=float_dtype) | |||
There was a problem hiding this comment.
LGTM to me also.
Just FYI, I'll note that in some places using float(dt) may also make sense to ensure "weak" promotion handling (i.e. float scalars behave like they used to in most cases on both NumPy 1 and 2).
I don't think this needs a fix here!
There was a problem hiding this comment.
Yeah I think here we realized that we were passing a Python float to a CuPy-based custom kernel and relying on whatever default promotion it was doing, which CuPy changed recently. There's more context in 3 under comment: #742 (comment)
So we decided it was better that we determine the type here instead. Hence the change
Hopefully that context is helpful
There was a problem hiding this comment.
Ah, NumPy 2 + CuPy 13.2 interaction might make promotion a bit more tricky even, so I guess the Python float thing may not work there.
There was a problem hiding this comment.
Yeah actually in the case where the error was seen, we are using the default value for dt (as it is unspecified)
Looking at the default value for dt, it appears to be 0.5
So perhaps CuPy has decided to always promote float to float64 before CUDA kernel calls
Greg did the investigation here. So likely knows more about the issue
There was a problem hiding this comment.
All, good, I don't think it's worth to track this down exactly. Eventually (probably with CuPy 14), this shouldn't be necessary for Python floats at least. But it is definitely the pragmatic thing right now.
|
/merge |
|
Thanks Greg and Sebastian! 🙏 |
jakirkham
changed the title
|
Fixed a minor typo in the PR title and in the CHANGELOG. This was picked up by a style check in 24.10 xref: #759 |
This MR resolves issues 3 and 4 reported here when testing with NumPy 2.0 and CuPy 13.2 locally (issues 1 and 2 will be addressed in CuPy itself)
#742 (comment)
For now, leave NumPy pinning as-is. Tests should continue to pass with NumPy 1.x