Array stacking

[vbarrielle]

This is a first draft for array stacking. Currently it does not work on anything else but slices of ArrayView.

Not sure about the performance of the algorithm, it probably depends on the datalayout. Maybe we want some special handling for common paths.

Status:

• basic implementation
• comprehensive tests
• macro support
• broadcasting support

Fixes #75.

[bluss]

Maybe the From/AsArray trait in #116 helps. This is going to be somewhat clumsy anyway, to make array types line up unfortunately.

I couldn't help but try this, because it seems so simple, and it assigns in larger chunks:

        let mut assign_view = result.view_mut();
        for array in &arrays {
            let len = *array.dim().index(axis);
            let (mut front, rest) = assign_view.split_at(axis, len);
            front.assign(&array);
            assign_view = rest;
        }

If we are using the trait Zero to initialize, I also want to consider using the trait Copy instead. I don't know if that's a too harsh restriction, but it allows using uninitialized data in the array without too much trouble.

[vbarrielle]

This is much simpler than what I came up with. And probably more performant too. I'll switch my impl to that. A copy bound looks nice too, but I'm not sure how I should try and avoid initializing data. If I remember correctly from the rustonomicon, there's quite a few guarantees to uphold when dealing with uninitialized data. But they're probably weaker for copy types. What ere they exactly?

[bluss]

Copy precludes destructors, which is the nice thing.

The only thing that must be upheld is that uninitialized values must not be read; this means the function must guarantee that all elements have been written before passing the array to the user.

Overwriting like .assign does (*x = y.clone();) is fine. There's no panic safety issue either, a vec can be dropped with uninitialized elements if they are Copy.

[bluss]

I would love to provide a constructor called empty like numpy does that produces uninitialized values, but it's unfortunately not safe rust. Maybe we'll get such a constructor (marked unsafe) down the line. For now I'm fine with forcing users to do it the long way or the safe way.

You can have an uninit array like this:

    let mut v = Vec::with_capacity(size);
    unsafe {
        v.set_len(size);
    }

    let mut result = OwnedArray::from_vec_dim(result_dim, v).unwrap();

[bluss]

• Can this be a single free function?
• We can use a Result<_, ShapeError> return
• We can add a macro that uses the AsArray conversion trait to make it simple.
• The macro can .unwrap() by default? If the free function / base API exists with the full control api.

[vbarrielle]

Some updates:

• used your split_at implementation
• require Copy bound and no zero-initialization
• now stacking is a free function, returning a Result.

I'll take a look at a macro and the AsArray trait.

[bluss]

ok, I merged the From/AsArray changes. Say if you want those impls changed, or if they're not perfect for this.

[bluss]

Accepting only &[ArrayView] but having a macro convert each argument to that could be a useful formulation.

The thing the macro does is primarily to accept multiple values of different types.

[bluss]

This is in fact overly strict -- assign allows broadcasting.

[vbarrielle]

Right, I had forgotten about broadcasting, I'll have to think about it.

[vbarrielle]

Here's a first attempt at a macro for stacking. I could not manage to use the AsArray trait though, I supposed I would call .into() here instead of view(), but that does not seem to work that easily. I haven't thought about it that much right now though.

[bluss]

I think using the new ArrayView::from works well. Shouldn't do much different than the .view() call. But .view() works well too? View accepts all kinds of arrays and references to arrays.

Should we make the syntax lighter? stack![Axis(0), a, b, c] would work too I think. And using + to enforce that there is at least one array.

[vbarrielle]

I think using the new ArrayView::from works well. Shouldn't do much
different than the .view() call. But .view() works well too? View accepts
all kinds of arrays and references to arrays.

Yes .view() works too. But maybe ArrayView::from is more generic ie it's
open for extension from the trait.

Should we make the syntax lighter? stack![Axis(0), a, b, c] would work too I
think. And using + to enforce that there is at least one array.

Yes looks nice that way.

[vbarrielle]

After trying, using ArrayView::from requires the user to import ArrayView at the call site, while using view() works without such requirement. So maybe it's better to use view()?

[bluss]

that part is easy to fix by using $crate::ArrayView::from i.e. the full path. Apart from that, I'm open to any feedback on how the from trait actually works/doesn't work for this.

[vbarrielle]

The full path resolves, alas there are still issues:

error: the trait `core::convert::From<ndarray::ArrayBase<collections::vec::Vec<_>, (usize, usize)>>` is not implemented for the type `ndarray::ArrayBase<ndarray::ViewRepr<&_>, _>`

[bluss]

That's a good error, since we should not consume an OwnedArray just to create a view from it. We can solve it later.

[vbarrielle]

Right, so there's no problem when passing a reference to the macro. Maybe that's better than .view()then, makes it clearer that the ownership isn't taken?

[bluss]

This is actually just arrays[0].ndim() or res_dim.ndim()

[vbarrielle]

I've cleaned up according to your remarks, and added some more tests.

[bluss]

It's really nice. We can solve the remaining things later, everything looks solid & broadcasting is easy to add later.

[vbarrielle]

The latest version uses ArrayView::from in the macro. If the current state is ok we can merge. I can always revert the last commit if you prefer the .view() version.

[bluss]

ArrayVec::from(& $argument) with an implicit & will also work. That will make the macro a bit more magic (no explicit borrows, just like println).

[bluss]

Thanks for working on this, it turned out really nice.

It's way past time to release 0.4.0 now..