Implement trait on Owned and View arrays

[davenza]

Hi,

this could be more of a Rust question than ndarray's question, but I submit this issue because maybe some of you have solved this yet.

I have an array inside of a struct Wrapper. The array could be owned or view. Now, I am trying to implement a given operation that requires using into_subview() for both owned and view Wrapper structs. My take on this was:

use ndarray::{Array, ArrayBase, Axis, IxDyn, OwnedRepr, ViewRepr, Data};
use std::mem;

pub struct Wrapper<S>
    where S: Data
{
    a: ArrayBase<S, IxDyn>
}

pub trait Operation {
    fn op(&mut self, axis: Axis, index: usize);
}

impl<V> Operation for Wrapper<OwnedRepr<V>>
    where OwnedRepr<V> : Data<Elem=V>,
{
    fn op(&mut self, axis: Axis, index: usize) {
        let mut owned = mem::replace(&mut self.a,
                                     Array::from_vec(Vec::new()).into_dyn());
        owned = owned.into_subview(axis, index);
        mem::replace(&mut self.a, owned);
    }
}

impl<'a, V> Operation for Wrapper<ViewRepr<&'a V>>
    where ViewRepr<&'a V> : Data<Elem=V>,
{
    fn op(&mut self, axis: Axis, index: usize) {
        let empty_array = Array::from_vec(Vec::new()).into_dyn();
        let empty_view = empty_array.view();
        
        let mut owned = mem::replace(&mut self.a,
                                     empty_view);
        owned = owned.into_subview(axis, index);
        mem::replace(&mut self.a, owned);
    }
}

This code does not compile, it errors in the view implementation:

error[E0597]: empty_array does not live long enough                                                                                                                                                              
  --> src/mwe.rs:30:26
                                                                                                                                                                         
   |                                                                                                                                                                                                               
30 |         let empty_view = empty_array.view();                                                                                                                                                                  
   |                          ^^^^^^^^^^^ borrowed value does not live long enough                                                                                                                                 
...                                                                                                                                                                                                                
36 |     }                                                                                                                                                                                                         
   |     - borrowed value only lives until here                                                                                                                                                                    
   |                                                                                                                                                                                                               
note: borrowed value must be valid for the lifetime 'a as defined on the impl at 25:6...                                                                                                                           
  --> src/mwe.rs:25:6                                                                                                                                                                                  
   |                                                                                                                                                                                                               
25 | impl<'a, V> Operation for Wrapper<ViewRepr<&'a V>>                                                                                                                                                            
   |      ^^                                                                                                                                                                                                       
                                                                                                                                                                                                                   
error: aborting due to previous error                                                                                                                                                                              
                                                                                                                                                                                                                   
For more information about this error, try `rustc --explain E0597`. 

At first, I thought that the mem::replace() in the view implementation was bounding the lifetime of the empty_view. However, it fails the same with the mem::replace() code commented:

impl<'a, V> Operation for Wrapper<ViewRepr<&'a V>>
    where ViewRepr<&'a V> : Data<Elem=V>,
{
    fn op(&mut self, axis: Axis, index: usize) {
        let empty_array = Array::from_vec(Vec::new()).into_dyn();
        let empty_view = empty_array.view();

//        let mut owned = mem::replace(&mut self.a,
//                                     empty_view);
//        owned = owned.into_subview(axis, index);
//        mem::replace(&mut self.a, owned);
    }
}

How would you implement this (if possible)? I don't get why it is failing.

Thanks.

[davenza]

I reviewed my problem and I think that the best solution is to clone() the view:

impl<'a, V> Operation for Wrapper<ViewRepr<&'a V>>
    where ViewRepr<&'a V> : Data<Elem=V>,
{
    fn op(&mut self, axis: Axis, index: usize) {
        let cloned_a = self.a.clone();
        self.a = cloned_a.into_subview(axis, index);
    }
}

This code just clones the shape and strides and reuses the data, so I think is not too bad.

---

However, I was thinking, why does into_subview() need ownership over self?

Reviewing the code, into_subview() calls subview_inplace() and remove_axis():

    pub fn into_subview(mut self, axis: Axis, index: Ix) -> ArrayBase<S, D::Smaller>
        where D: RemoveAxis,
    {
        self.subview_inplace(axis, index);
        self.remove_axis(axis)
    }

subview_inplace() needs only &mut self:

    pub fn subview_inplace(&mut self, axis: Axis, index: Ix) {
        dimension::do_sub(&mut self.dim, &mut self.ptr, &self.strides,
                          axis.index(), index)
    }

remove_axis() needs ownership over self:

    pub fn remove_axis(self, axis: Axis) -> ArrayBase<S, D::Smaller>
        where D: RemoveAxis,
    {
        assert!(self.ndim() != 0);
        let d = self.dim.remove_axis(axis);
        let s = self.strides.remove_axis(axis);
        ArrayBase {
            ptr: self.ptr,
            data: self.data,
            dim: d,
            strides: s,
        }
    }

but this code only calls to RemoveAxis::remove_axis(), and RemoveAxis::remove_axis() only needs &self:

pub trait RemoveAxis : Dimension {
    fn remove_axis(&self, axis: Axis) -> Self::Smaller;
}

I assume that must be a good reason to require self, but I don't get it.

[jturner314]

I reviewed my problem and I think that the best solution is to clone() the view

I agree. Another option would be to use the take_mut crate which allows temporarily taking ownership of a value from behind a mutable reference, but I think there's some question whether doing that is sound.

I assume that must be a good reason to require self, but I don't get it.

If I understand correctly, you're asking why we can't do this:

    pub fn into_subview2(&mut self, axis: Axis, index: Ix) -> ArrayBase<S, D::Smaller>
    where
        D: RemoveAxis,
    {
        // implementation
    }

It would be possible to implement a method with that kind of signature if we clone things:

    pub fn into_subview2(&mut self, axis: Axis, index: Ix) -> ArrayBase<S, D::Smaller>
    where
        S: DataClone,
        D: RemoveAxis,
    {
        let mut c = self.clone();
        c.subview_inplace(axis, index);
        c.remove_axis(axis)
    }

However, if we want to avoid cloning, then it's not possible to implement this such that the result has the same elements as the input. Consider the case where self is an owned array (Array<A, D>). Then, the method would receive a &mut Array<A, D> and need to return an Array<A, D>. We can't just make the returned owned array point to the same elements as the input, because that would violate Rust's aliasing rules. (We'd be creating an owned array that points to the same data as the mutable borrow.)

Alternatively, you might be asking why we can't do this:

    pub fn into_subview2(&mut self, axis: Axis, index: Ix)
    where
        D: RemoveAxis,
    {
        // implementation
    }

where, unlike .subview_inplace(), this would remove the axis. We can't do this because the number of axes is part of the type of the array, and it's not possible to change the type of something given only a reference to it.

Note, however, that it would be possible for us to provide a special case of this for IxDyn:

impl<A, S> ArrayBase<S, IxDyn>
where
    S: Data<Elem = A>,
{
    pub fn into_subview2(&mut self, axis: Axis, index: Ix) {
        // implementation
    }
}

because removing axes does not change the type of the array. (It's still IxDyn.) We just haven't added a method like this. What would be a good name for it?

Does this answer your question?

[davenza]

Thanks for your answer, @jturner314 .

where, unlike .subview_inplace(), this would remove the axis. We can't do this because the number of axes is part of the type of the array, and it's not possible to change the type of something given only a reference to it.

That answers my questions about remove_axis().

Note, however, that it would be possible for us to provide a special case of this for IxDyn:

impl<A, S> ArrayBase<S, IxDyn>
where
    S: Data<Elem = A>,
{
    pub fn into_subview2(&mut self, axis: Axis, index: Ix) {
        // implementation
    }
}

because removing axes does not change the type of the array. (It's still IxDyn.) We just haven't added a method like this. What would be a good name for it?

Does this answer your question?

I think this (#533) is nice.

[jturner314]

ndarray 0.12.1 has an index_axis_inplace method for dynamic-dimensional arrays. Does this method solve the issue?

[davenza]

@jturner314 Thanks! It solves the issue.