Reshape uses virtual view instead of deep copy - #90
Open
konstibob wants to merge 1 commit into
Open
Conversation
normanrz
approved these changes
Sep 4, 2026
Comment on lines
+90
to
+112
| * | ||
| * <p>The two obvious candidates are both wrong here. {@link Array#reshape} always allocates and | ||
| * copies. {@link Array#reshapeNoCopy} hands the raw backing store to the new shape and discards | ||
| * the input's strides and offset, so it silently reorders the elements of any view — such as | ||
| * the output of the {@code transpose} codec, or the strided section that {@code Array.write} | ||
| * passes in for every chunk of a multi-chunk write. | ||
| * | ||
| * <p>{@link Array#get1DJavaArray} instead returns the backing store itself when the input already | ||
| * walks it in lexicographical order (ma2 tracks this as {@code Index.fastIterator}), and a C-order | ||
| * copy when it does not. The result therefore shares its storage with the input whenever the | ||
| * elements are already laid out in {@code ravel} order, and is a correct copy otherwise. Either | ||
| * way {@code ravel(B) == ravel(A)} holds, and the reshaped array is itself in lexicographical | ||
| * order, so a following codec gets the cheap path too. | ||
| * | ||
| * <p>This is conservative compared to NumPy's {@code _attempt_nocopy_reshape}, which also keeps a | ||
| * view when splitting the axes of a strided array, and when the axes being merged happen to be | ||
| * internally contiguous ({@code stride[k] == shape[k+1] * stride[k+1]}). Matching that would need | ||
| * an {@link ucar.ma2.Index} with custom strides and a non-zero offset, which ma2's public API | ||
| * cannot build safely: {@code new Index(shape, stride)} forces {@code offset = 0} and leaves the | ||
| * internal {@code fastIterator} flag set, so the resulting array would later hand out its whole | ||
| * backing store as if it were the data. Decode is unaffected either way, because it always | ||
| * receives a freshly allocated array; on encode the extra copy is limited to multi-chunk writes, | ||
| * where the caller passes a strided section. |
Member
There was a problem hiding this comment.
Suggested change
| * | |
| * <p>The two obvious candidates are both wrong here. {@link Array#reshape} always allocates and | |
| * copies. {@link Array#reshapeNoCopy} hands the raw backing store to the new shape and discards | |
| * the input's strides and offset, so it silently reorders the elements of any view — such as | |
| * the output of the {@code transpose} codec, or the strided section that {@code Array.write} | |
| * passes in for every chunk of a multi-chunk write. | |
| * | |
| * <p>{@link Array#get1DJavaArray} instead returns the backing store itself when the input already | |
| * walks it in lexicographical order (ma2 tracks this as {@code Index.fastIterator}), and a C-order | |
| * copy when it does not. The result therefore shares its storage with the input whenever the | |
| * elements are already laid out in {@code ravel} order, and is a correct copy otherwise. Either | |
| * way {@code ravel(B) == ravel(A)} holds, and the reshaped array is itself in lexicographical | |
| * order, so a following codec gets the cheap path too. | |
| * | |
| * <p>This is conservative compared to NumPy's {@code _attempt_nocopy_reshape}, which also keeps a | |
| * view when splitting the axes of a strided array, and when the axes being merged happen to be | |
| * internally contiguous ({@code stride[k] == shape[k+1] * stride[k+1]}). Matching that would need | |
| * an {@link ucar.ma2.Index} with custom strides and a non-zero offset, which ma2's public API | |
| * cannot build safely: {@code new Index(shape, stride)} forces {@code offset = 0} and leaves the | |
| * internal {@code fastIterator} flag set, so the resulting array would later hand out its whole | |
| * backing store as if it were the data. Decode is unaffected either way, because it always | |
| * receives a freshly allocated array; on encode the extra copy is limited to multi-chunk writes, | |
| * where the caller passes a strided section. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
added mechanism similar to https://github.com/numpy/numpy/blob/main/numpy/_core/src/multiarray/shape.c
to construct a virtual view instead of copying when possible, only adjusting the shape and stride parameters in the indexCal Parameter of a ma2.Array