-
-
Notifications
You must be signed in to change notification settings - Fork 7.8k
Use old stride_windows implementation on 32-bit builds #29115
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
base: main
Are you sure you want to change the base?
Conversation
This was originally for i686 on Fedora, but is now applicable to WASM, which is 32-bit. The older implementation doesn't OOM.
It's not really clear to me why sliding_window_view (in the way we use it) would lead to an OOM while the manual approach wouldn't? |
Perhaps there is a NumPy calculation bug? It ends up as:
|
Oh, I see, this is because the intermediate array is too large even though we slice it immediately (to compute the overlapping FTs); also it seems like numpy wants array.size * array.itemsize to be representable even though that may be much bigger than the physical array size. That seems overall related to the request for step_size at numpy/numpy#18244. I guess the easy way out is indeed to go back to as_strided. |
I thought mlab was being deprecated at some point. How useful is this to add this code piece, versus adding a |
Fair enough; I don't know what the status of the deprecations are at this point. I will say that this is reverting to the pre-#21190 code, so it's not new, and I've been using the patch on Fedora without issue since that PR, so it's been stable AFAICT. |
PR summary
I've long had the patch on Fedora (since #21190 (comment)), but it's now applicable to WASM as well (#29093), which is 32-bit. The older implementation doesn't OOM.
cc @anntzer as original author of that PR in case you have an alternate implementation idea.
PR checklist