Better reshaping, part 1: .to_shape()

[bluss]

New features:

• .to_shape(impl IntoDimension) -> CowArray
• .to_shape((impl IntoDimension, Order)) -> CowArray
• to_shape can return a view in all cases where it's technically possible to do so, saving copying. This is a big step forwards compared to previous into_shape and similar (and this improvement will eventually land in all the reshaping methods.)

use ndarray::{array, Order};

assert!(
    array![1., 2., 3., 4., 5., 6.].to_shape(((2, 3), Order::ColumnMajor)).unwrap()
    == array![[1., 3., 5.],
              [2., 4., 6.]]
);

• to_shape returns a view if possible otherwise copies into an owned array. Always succeeds if the number of elements is correct.

Method parameters:

• .to_shape<Sh>(shape: Sh) where Sh: ShapeArg

  • Takes a dimension pattern like (1, 2, 3) like before. This uses the default Order.
  • Also takes a tuple of dimension and order like ((1, 2, 3), Order::RowMajor) to specify order

• Order enum:

  • RowMajor (with alias C; this is the default if not specified)
  • ColumnMajor (with alias F)

Notes

It's important to have reshaping methods that can do so without any clone bound or similar. For example a reshape operation that can be used on everything from raw views, views and other arrays. into_shape or similar should fill this need.

Coming in a later change:

A version of into_shape that uses the Order parameter and that can always reshape owned arrays.

• .into_shape() -> "Self" (Same array storage type)

• into_shape transforms the input but can only do so (for views) if the memory layout is exactly correct. For Array and uniquely owned ArcArray, it always succeeds if the number of elements is correct.

Order::Automatic

Order::Automatic has been explored (and implemented) but is not part of this version of the change. No use case has been presented.

Partially addresses #390

[bluss]

@jturner314 What do you think about these aspects of the shape + order argument?

1. Use the same shape + order argument in .to_shape() as we do in for example Array::zeros
2. The vehicle for ordering can be the enum Order { RowMajor, ColumnMajor, Automatic } - with aliases Order::C, Order::F, Order::A (associated consts).

• Automatic could be removed if we don't like it. For construction it would have to mean "default".
• I think it is ok to "overload" the meaning of this to mean both memory layout and logical index order - we can understand those as two sides of the same coin

3. An important question is if the function is going to be called like a or b:
   a. .to_shape((2, 3).order(Order::RowMajor))
   b. .to_shape(((2, 3), Order::RowMajor))

• a is easier to read but requires us to continue the trait that adds methods to tuples, and .order(Order::F) is a bit redundant.
• b is simple and easy to understand - drawback is the forest of ()'s.
• At the moment I'm leaning towards b. Both could even be used - a could convert to b.
• For construction, custom strides needs to be factored in. Either (2, 3).strides(..) or ((2, 3), CustomStrides(..)) would work.

It would be fast to just add to_shape - but this PR is sort of delayed because of having to solve the new and better shape arguments for construction - replacing the old ones.

[jturner314]

Use the same shape + order argument in .to_shape() as we do in for example Array::zeros

I think this is fine. While it is true that the order parameter is being used in two different ways -- .to_shape() uses it as "the mapping from an n-D index to a 1-D logical index of the input array and output array" and Array::zeros uses it as "the mapping from an n-D index to a 1-D memory offset of the output array" -- I think that's okay.

The primary case where I could see us wanting different options for these two cases would be if we wanted to support directly specifying strides for Array::zeros for convenience (with a check that there are no holes). Allowing strides as a parameter for .to_shape() would be too confusing, IMO. However, we could handle this case in a nice way by adding a conversion from contiguous strides to the Order type.

Another case is that we may want to define a ContiguousLayout struct (with shape and strides/order fields) for various purposes, including constructors. The name ContiguousLayout would be confusing as a parameter for .to_shape(). I generally associate "layout" with "memory layout", but .to_shape() isn't really using the order to specify a memory layout. I don't think this is a significant concern, though.

The vehicle for ordering can be the enum Order { RowMajor, ColumnMajor, Automatic } - with aliases Order::C, Order::F, Order::A (associated consts).

I agree that an enum works well for this. By the way, the idea of using associated consts for enum variant aliases is pretty clever. I haven't seen that before.

• Automatic could be removed if we don't like it. For construction it would have to mean "default".

Personally, I don't think I'd ever use an Automatic option with reshaping, since it makes the output values depend on the memory layout of the input. NumPy has this option, though, so I'd guess that it has some users. My preference would be to leave it out for now, and add it only if someone requests it.

• I think it is ok to "overload" the meaning of this to mean both memory layout and logical index order - we can understand those as two sides of the same coin

Yes, I think it's okay to "overload" the meaning of the order. In both cases, it represents a mapping from an n-D index to a 1-D index. It's basically equivalent to strides, except for the lack of holes in the 1-D space.

I think that it would be worth adding a more flexible Axes order variant which has a list of axis indices, so, for example, [0, 1, 2] would be C layout for 3 dimensions and [2, 1, 0] would be F layout. I'd actually suggest adding an AxesOrder struct as a wrapper around D: Dimension to enforce uniqueness and in-boundedness. The Order::Axes enum variant would wrap the struct. However, this variant isn't necessary for an initial implementation of .to_shape().

We could also consider allowing axis inversions in the order, but that's less important than the "axes order" variant, since inversions don't change the shape. (It's simple to call invert_axis after constructing an array, for example. In contrast, a workaround for the lack of an Axes order variant requires reordering the shape passed to the constructor.) That said, it would be nice to support axis inversions in the order so that we could have an .order() method which would return the order of an array's elements.

An important question is if the function is going to be called like a or b:

a. .to_shape((2, 3).order(Order::RowMajor))
b. .to_shape(((2, 3), Order::RowMajor))

First, it's worth noting that these will be written as

a. .to_shape([2, 3].order(Order::RowMajor))
b. .to_shape(([2, 3], Order::RowMajor))

once we drop support for using tuples to represent shapes, so the parentheses in the b case aren't too bad. I don't have a strong preference. I like b a bit better because depending on an extension trait implemented for standard library types is a little unusual.

I wouldn't mind requiring the order parameter for .to_shape(), which would simplify things for it.

We could remove support for using [usize; N] directly as a shape, and instead require a Shape struct, which would remove the need for an extension trait to add an .order() method. I dislike this option, though, because it's much less concise than just using [usize; N] directly.

One more thought -- another use for the Order type is iteration. We could add an .iter_with_order() method, for example.

[bluss]

Thanks!

• to_shape/into_shape calls are so useful and common that I want to be able to use them without explicit ordering arguments.
• We'll use lots of examples where ordering is explicitly specified, to make it clear that this is also a normal use case.
• To avoid having builder structs for so many methods, let's try to use a ShapeArg or similar trait i.e. where the diversity of the method argument sits in the argument position. The upside is that the simple case is simple - just .to_shape((2, 3)) etc.
• I agree that we will get to a place where shapes are canonically specified with arrays - we will map [usize; N] <-> N-dimensional array shape. Then, why not keep the IntoDimension or similar implementations for tuples? I don't want to break all ndarray code in existance, and right now it looks like we can easily keep the tuple conversions too.

[jturner314]

• to_shape/into_shape calls are so useful and common that I want to be able to use them without explicit ordering arguments.

Okay, that's fine with me. I think the default should definitely be C order rather than e.g. an "automatic" (layout-dependent) order.

right now it looks like we can easily keep the tuple conversions too.

Yes, that will probably always be true for small ndims. I just assumed that we'd drop support for tuples at some point, because I don't see much reason to continue supporting them except for ease of upgrading to the next breaking release. Personally, I don't mind breaking changes in a breaking release as long as (1) there's at least a compiler warning (preferably an error) about the breakage and (2) it's easy to update the code. In the case of dropping tuple support, there will be a compiler error for each tuple, and the fix is to change () to [] wherever there's an error.

There are a few reasons to drop support for tuples:

1. It's less code to support.
2. If we support arbitrary const ndims once Rust has full const generics, we won't be able to support conversions from tuples for all supported ndims (unless Rust also adds some kind of variadic tuple support).
3. IMO, tuples are really intended for heterogeneous elements, so it's a bit awkward to use them as shapes.
4. It would force downstream crates to switch from tuples + arrays to just arrays, which may help slightly with compatibility between downstream crates.

[bluss]

I'm setting up this PR to be just the non-breaking addition of .to_shape(). I've implemented the into_shape() changes and prototyped future ShapeArg usages for constructors, so I'm ready for the future breaking changes too, to those, in a later PR.

What you said now brings up the future breaking change to into_shape(). I would suggest that .into_shape((2, 3)) changes meaning - from the current "automatic" layout inference to the new "default row-major"; this has the potential for being a silent breaking change for people, but I don't see a great way of fixing that.

[jturner314]

What you said now brings up the future breaking change to into_shape(). I would suggest that .into_shape((2, 3)) changes meaning - from the current "automatic" layout inference to the new "default row-major"; this has the potential for being a silent breaking change for people, but I don't see a great way of fixing that.

Oh, that's a good point. I agree that the order for into_shape should change from "automatic" to "default row-major".

We should do what we can to avoid a silent breaking change, because that could cause a real headache for someone using Fortran-layout arrays if they don't notice the relevant item in the release notes. I see a few options:

1. For the next breaking release or two, require the user to specify the order. For example, add a extra, temporary trait bound for .into_shape() in addition to ShapeArg which is implemented only for (shape, order). After a couple of releases, remove this extra bound.

2. For the next breaking release or two, panic or return an error if the user doesn't explicitly specify an order and the result would be different from what it is now. (In other words, check for what would otherwise be a silent change.) After a couple of releases, remove this check. The disadvantage of this option is that the panic/error is at runtime rather than at compile time.

3. For the next breaking release or two, deprecate .into_shape() and add a method with a new name. After a couple of releases, rename the new method to .into_shape() and deprecate the old name. The disadvantage of this option is that it requires the users to make two changes to their code (switch to the new method name for a while, and then switch back to .into_shape()).

I like option 1 the best, because it will result in a compilation error, and users will need to update their code only once (unless they use the temporary trait as a bound for something). Hopefully, if we keep the mitigation in place for a couple of breaking releases, we'll catch the vast majority of users.

[bluss]

It seems there is no perfect solution for into_shape, that both has the breaking change but also helps the users.

I'm contemplating now your solution (1) with the addition of an opt-in feature flag - into_shape_row_major_default that the user can enable to explicitly enable the new default. Without the feature flag, only explicit order works. They can acknowledge the breaking change by enabling the feature flag, and then we deprecate and remove the feature it in a later release. Convoluted, but maybe that's the best possible solution?

[jturner314]

The feature flag is a good idea. The primary issue is that if any crate in the build enables the feature, all crates depending on that version of ndarray will be affected. So, if ndarray-linalg enables the feature, for example, users of ndarray-linalg and ndarray won't have to explicitly enable the feature. So, if we go this route, we should strongly recommend that only the final binary crate (i.e. not library crates) enable the feature.

[SparrowLii]

Just have a question: as for non-standard C or Fortran arrays, can Order become smarter? Look at the following examples:

use ndarray::{Array, ShapeBuilder};
fn main() {
    let data = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16];
    let a = Array::from_shape_vec((2, 4, 2).strides((1, 4, 2)), data.to_vec()).unwrap();

    // Unchanged
    let a1 = a.to_shape((2, 4, 2)).unwrap();

    // Add a dimension
    let a2 = a.to_shape((1, 2, 4, 2)).unwrap();

    // Expand a certain dimension
    let a3 = a.to_shape((2, 2, 2, 2)).unwrap();
}

In these cases, it is completely feasible to output an ArrayView instead of a new Array. Therefore we can avoid array traversal, and these cases can also be supported in into_shape.

[bluss]

Yes, that's certainly a desired feature in to_shape and into_shape if it's not too complicated to implement. It's something that can be added after these initial implementations. For changes like (2, 4, 4) -> (2, 2, 2, 4) or similar, it's important that they are defined by an unambiguous rule and that we can do them in a way that's consistent with the selected index ordering.

I think a rule can be developed, I'd see that you walk through the incoming and outgoing dimensions and strides and "consume" them part by part and build up the corresponding stride sequence for the new shape. Potentially starting in the front/back depending on if it's C or F ordering.

[SparrowLii]

I thought about this problem again, and I think it can be implemented according to the following rules:
Take Order::RawMajor for example, we traversing the incoming and outgoing dimensions from back to front. This matter will encounter the following situations:

1. incoming or outgoing is 1

2. incoming and outgoing are equal

3. incoming is divisible by outgoing
   For example, [4] -> [2, 2]. It means that we need to expand the original dimension

4. incoming cannot be divisible by outgoing
   For example, [2, 2] -> [4] or [2, 3] -> [3, 2].
   At this time, we try to merge the last two dimensions of incoming. If it cannot be merged, it means that reshape cannot be performed. Then we enter the next cycle.
   Because the total number of elements of incoming and outgoing is the same, finally incoming can always be divisible by outgoing.
   Here are some of my attempts, but obviously it needs more test cases.

[jturner314]

I'd suggest a preprocessing step on the input array to merge as many axes as possible toward the "inner" axes (according to the desired order). I think @bluss is adding functionality like this in #979. Then, you don't have to worry about merging axes of the input at the same time as dealing with the output shape/strides.

[bluss]

Actually as soon as I had mentioned some details I went to trying to implement it, so I have a work in progress going too. i think it's already working for both C/F order and quite general (can handle things like 3, 4, 5 to 15, 4 and so on). We can compare code when I have time to post. It's a relief to get this functionality.

[bluss]

This is ready to merge