Skip to content
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

Feature request: support out argument in reshape #7488

Open
coderforlife opened this issue Mar 31, 2016 · 10 comments
Open

Feature request: support out argument in reshape #7488

coderforlife opened this issue Mar 31, 2016 · 10 comments

Comments

@coderforlife
Copy link

The out argument would be used for the output copy. This means that the array will always be copied, even if it could be reshaped without copying, thus becoming the opposite of doing a.shape = (...) which never copies. This would allow one to copy the data where both the array being reshaped and the destination array cannot be reshaped to the other without a copy being made and not make an intermediate. This would also give options for emulating repeat and tile with an out argument by using as_strided and reshape.

@njsmith
Copy link
Member

njsmith commented Mar 31, 2016

It sounds like your use cases would be better served by a copy=True option? The awkwardness with out= is that you'd have to pre-allocate an appropriately sized array.

@njsmith
Copy link
Member

njsmith commented Mar 31, 2016

Or maybe copy="always" | "never" | "if needed"

@coderforlife
Copy link
Author

I do have an appropriately-sized array. It is potentially very large (4+ GiB) and is based on shared-memory instead of the standard memory backing, so a regular copy would not be helpful, just as will other memory backings, like memmapped files. So, for my needs, a copy=True option would actually be inappropriate. I use the out argument frequently with so many other functions to keep intermediates at a minimum, seeing that my intermediates can be quite massive.

@seberg
Copy link
Member

seberg commented Mar 31, 2016

Its orthogonal, but as a workaround you can reshape the out array instead of the input array:

# out_arr.shape == desired_shape
out_arr.reshape(old_arr.shape)[...] = old_arr

of course that gives a little restriction on the out arrays strides maybe, but in practice you probably don't care about that.

@coderforlife
Copy link
Author

For the mean time I am doing something similar to that, but catching the AttributeError raised by out_arr.shape = old_arr.shape and falling back to doing out_arr.flat = old_arr. The fallback is quite slow, it usually takes more than twice as long as doing copyto(out_arr, old_arr.reshape(out_arr.shape)) which does 2 complete copies and an allocation, but at least it doesn't do the intermediate allocation! I wish that the undocumented and unimplemented PyArray_AssignArrayAsFlat method was available, it seems to be the solution.

@seberg
Copy link
Member

seberg commented Apr 1, 2016

The copyto code does not do two copies, it just does one copy? out_arr.shape = old_arr.shape will do the copy, and then fail though.

@coderforlife
Copy link
Author

copyto(out_arr, old_arr.reshape(out_arr.shape)) does two copies, one from old_arr to the intermediate, then another from the intermediate to out_arr. Looking in PyArray_NewShape there is a PyArray_NewCopy call which is done in this case because old_arr can't be reshaped in place.

My complete solution for the moment is as follows:

try:
    # If we can reshape out_arr, do it (frequently this is possible, but not always)
    out_view = out_arr.view()
    out_view.shape = old_arr.shape
    copyto(out_view, old_arr)
except AttributeError:
    # Fallback if we can't reshape out_arr
    out_arr.flat = old_arr

Remember, it is old_arr that can't be ever reshaped in place.

An alternative to having an out argument in reshape is to implement PyArray_AssignArrayAsFlat and then expose this through copyto or similar, allowing any two arrays that have the same size. This might be a better solution overall as it seems this feature was planned at some point, just never made it off the ground.

@seberg
Copy link
Member

seberg commented Apr 1, 2016

@coderforlife yes right. I read it the other way around, like you have in your complete solution there, where you try to reshape the out array to the old arrays shape, which should usually work. One thing is that even for your hack, a copy="never" argument would make the try block look nicer (note that the try block will even take a similar time on failure as on success in current numpy).

The PyArray_AssignArrayAsFlat might look nice on first sight, but I think it is the same as arr.flat = ..., mapping to the old iterator. This will mean that things are not a strided copy, but copying element by element, which as you noticed is quite a lot slower then the reshape. Of course maybe we can try to fix PyArray_AssignArrayAsFlat's speed. You could even write your own assign array as flat manually, which should work fine for large arrays:

Since, it is crazy, this pure python function might be almost as fast as the successful try block and probably a lot faster then that flat assignment:

def assign_as_flat(src, dest, order='C'):
    if src.size != dest.size:
        raise ValueError("arrays have different size.")

    i1 = np.nditer(src, order=order, flags=['zerosize_ok', 'buffered', 'external_loop'])
    i2 = np.nditer(dest, order=order, flags=['zerosize_ok', 'buffered', 'external_loop', 'refs_ok'], op_flags=["writeonly"])

    for s, d in zip(i1, i2):
        d[...] = s

@seberg
Copy link
Member

seberg commented Apr 1, 2016

Actually I realized there is a CopyAsFlat or so in the C-api as well (not sure whether exposed, or fast, or...). I guess these are all real options that could be nice if anyone actually does them.

@coderforlife
Copy link
Author

Sorry for the delay, I finally got around to doing some testing.

My current version of Numpy doesn't have CopyAsFlat, so I haven't checked that out yet, but can soon. However, I did notice that in the most recent version of Numpy that CopyAsFlat is the same as CopyAnyInto for C-ordered data (another undocumented function, and it appears that CopyAsFlat used to be called CopyAnyIntoOrdered).

Also, back when I originally did this, I tried to make an nditer version but couldn't get it to work. I had not thought of using two iterators each having the same buffer size to make it work.

In any case, for some examples I get the following timings:

old.reshape(out.shape)                121 ms    (doesn't place in out, but using as a benchmark)
copyto(out, old.reshape(out.shape))   149 ms    (creates an intermediate copy)
out.shape=old.shape;copyto(out,old)    74.1 ms  (doesn't work if out cannot be reshaped)
nditer (as above)                      88.5 ms
CopyAnyInto(out, old)                 102 ms
out.flat = old                       1160 ms

CopyAnyInto is significantly better than the out.flat = old for universally copying the data (over 11x faster!), but using nditer is even better (and still universal) and finally the in-place reshape trick is the best (but is not universal).

I will try to version to see if CopyAnyInto got any improvements when being converted into CopyAsFlat because it really should not be slower than the nditer approach. (Note: my CopyAnyInto function was a simple Cython wrapper around the Numpy function).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants