don't copy data when changing shape #145
Conversation
as per the comment in shape.c, the use of PyArray_Reshape is not recommended. Change set_shape accordingly in getset.c.
If a shape of an array is set, but the shape is incompatible, the array is copied, and the copy immediately discarded. This commit changes this behaviour so that no copy is ever generated.
Move code of PyArray_Newshape and PyArray_Newshape_Nocopy into one function.
| if (ret == NULL) { | ||
| return -1; | ||
| if (!PyArray_IntpConverter(val, &newdims)) { | ||
| return -1; |
There was a problem hiding this comment.
Indentation. Are you using tabs by any chance? Wrong indent setting?
|
Can this be done by adding a function that checks if the array can be reshaped in place? Something like I suppose there also needs to be a function that also checks if the array can be reshaped with a copy. |
Accidentally, I was using tabs not spaces. (Yes, I do have correct tab settings, tab is eight characters, but yes, spaces are better anyhow).
Instead of introducing a new function PyArray_Reshape_Nocopy, the original function PyArray_Reshape gets extended. The parameter `order` may be set to a new value `NPY_NOCOPY`, which prevents PyArray_Reshape from copying.
certainly, it can be done this way, but I don't think it's a good |
| NPY_KEEPORDER=2, | ||
| /* Don't copy data, so order obviously is kept. | ||
| * used as special value in PyArray_Newshape */ | ||
| NPY_NOCOPY=3 |
There was a problem hiding this comment.
I think this may be a bad idea. This is a general enum in the publicly exposed API, so a special value for one API function isn't appropriate. Further, NOCOPY seems to have roughly the same meaning as KEEPORDER.
There was a problem hiding this comment.
No, NOCOPY has a very different meaning from KEEPORDER. The latter means that the order ought to
be kept (hence the name) while copying, while NOCOPY means that the data should not be copied.
|
This looks like a worthy bug to fix, but I don't think the way it's being fixed is good. Fixing it should not require changing the exposed NumPy API. |
|
So, I just changed this pull request not to expose NPY_NOCOPY into |
|
I agree with tecki that this implementation of array_shape_set needs improvement so that the copy does not happen if it doesn't need to. But, there should be some function that tests this (it looks like there is a related function to such a function in current master _attempt_nocopy_reshape). The NOCOPY flag to NewShape is an approach --- perhaps called KEEPORDER_AND_DATA |
|
I do think it makes sense to fix. The |
|
Closing on account of stale. @seberg Thoughts? |
feat: Add vrshr_n_s8
When changing the shape of an array, by writing, say,
Under the hood, however, what's happening is that
in this case the array is copied as it would be
if one had written
I discovered that while working with humongous
matrices, which nearly fill my memory. I used
the a.shape=something method because it
was supposed to be a memory no-op. Unfortunately
it isn't, as numpy tried and failed to copy the array
and raised a MemoryError instead of an AttributeError.
I consider this all a bug, not a feature, so I fixed it.
With this pull request, no data is ever copied when
changing the shape of an array.