Ndarray reshape

#1

I have something like this:

use ndarray::{Array, IxDyn};

fn reshape(array: &mut Array<f32, IxDyn>, shape: Vec<usize>) {
    // reshape array here
}

The problem is that Array::into_shape takes the array by value. So my first attempt was:

use ndarray::{Array, IxDyn};

fn reshape(array: &mut Array<f32, IxDyn>, shape: Vec<usize>) {
    array.view_mut().into_shape(shape.clone()).unwrap();
}

But this only creates a temporary view into the array and changes the shape only for the view. So I have to do something like this:

use ndarray::{Array, IxDyn};

fn reshape(array: &mut Array<f32, IxDyn>, shape: Vec<usize>) {
    *array = array.view_mut().into_shape(shape.clone()).unwrap().into_owned();
}

But I think this clones the whole array which I was trying to avoid…

#2

If you cannot change the reshape signature to take and return the array by value you could instead use std::mem::swap to swap the &mut Array handed to your function with a temporary empty array, call into_shape and then swap it back.

#3

Ok, thank you. Changing the signature is not an option in my case.
So I tried the mem::swap option. It does compile but I am not sure if it is correct implemented:

use std::mem;
use ndarray::{Array, IxDyn};

fn reshape(array: &mut Array<f32, IxDyn>, shape: Vec<usize>) {
    unsafe {
        let mut x = mem::uninitialized();

        mem::swap(array, &mut x);
        x = x.into_shape(shape.clone()).unwrap();
        mem::swap(array, &mut x);

        mem::forget(x);
    }
}

I think this function will be performance critical, so I don’t want to create a temporary array.
Or should I go for the temporary value and trust the compiler for optimizing it?

#5

@ehsanmok This code is incorrect, you must not drop the uninitialized memory. Therefore your code is unsound. @jan17’s code is correct as long as into_shape(...).unwrap() has no chance to panic, and will trigger UB if reshape into_shape(...).unwrap() does panic.

3 Likes
#6

It actually depends! that’s why I renamed it to inplace_reshape. This is a common practice in HPC afaik, which is a different world! Regular reshape would return and not drop of course.

#7

No, let’s trace your code

fn inplace_reshape(array: &mut Array<f32, IxDyn>, shape: Vec<usize>) {
    let mut ret = unsafe { Array::uninitialized(shape) }; // create uninit
    mem::swap(&mut ret, array);                           // swap uninit with input
} // overall, this code swaps uninit with the input and drops the input

fn main() {
    let mut a = Array::zeros((2, 3)).into_dyn(); // create array
    inplace_reshape(&mut a, vec![3, 2]);         // swap a with uninit and drop array
                                                 // note that at this point a == uninit
    println!("{:?}", a.shape());                 // UB
}

Now for @jan17’s code

use std::mem;
use ndarray::{Array, IxDyn};

fn reshape(array: &mut Array<f32, IxDyn>, shape: Vec<usize>) {
    unsafe {
                                          // *array == old_array_value
        let mut x = mem::uninitialized(); // x == uninit

        mem::swap(array, &mut x);                 // x == old_array_value and *array == uninit
        x = x.into_shape(shape.clone()).unwrap(); // note: branch
                                                  // on panic: drop(x), drop(*array)
                                                  // drop(*array) is UB because *array == uninit
                                                  // no panic: continue x == reshaped_array
        mem::swap(array, &mut x);                 //  x == uninit, *array == reshaped_array

        mem::forget(x);                           // forget x, so that it doesn't drop and cause UB
    }
}
1 Like
#8

No! please run it locally (wished play could import ndarray!). I’ve done it and I don’t post code here untested. Note that Array::uninitialized is different from mem::uninitialized.

#9

Running code is not a test for UB, I looked through the source code of Array::uninitialized before posting. They use Vec::set_len after setting the capacity of the vec with Vec::with_capacity. This is effectively the same as using std::mem::uninitialized.

2 Likes
#10

I’d like to here @bluss on this and what’d be the best practice for this inplace op.

#11

Also, the code does not appear to do what OP wants? They want to reshape the array, which presumably means they want to preserve whatever data was in the array prior to reshaping.

1 Like
#12

You mean inplace op doesn’t preserve the data?

#13

If you have to create a new array, you might as well clone().into_shape().

1 Like
#14

As I wrote in the trace of your code, inplace_reshape does not do anything but swap out what is referenced to by array with uninitialized. Nothing else. I think you meant to put more, but didn’t copy over the code correctly.

#15

I think the point is not to clone.

#16

Thank you all for your input so far.
Because into_shape can indeed panic, I came up with the following:

fn reshape_inplace<T>(array: &mut Array<T, IxDyn>, shape: Vec<usize>) {
    use std::mem;

    unsafe {
        let mut tmp = mem::uninitialized();

        mem::swap(array, &mut tmp);
        tmp = match tmp.into_shape(shape.clone()) {
            Ok(tmp) => tmp,
            Err(e) => {
                mem::forget(array);
                Err(e).unwrap()
            }
        };
        mem::swap(array, &mut tmp);

        mem::forget(tmp);
    }
}

Edit: I am sorry this does not compile right now, just as an idea how to handle the panic.

#17

close

fn reshape_inplace<T: Default>(array: &mut Array<T, IxDyn>, shape: Vec<usize>) {
    use std::mem;

    unsafe {
        let mut tmp = mem::uninitialized();

        mem::swap(array, &mut tmp);
        tmp = match tmp.into_shape(shape.clone()) {
            Ok(tmp) => tmp,
            Err(e) => {
                // before, it would just forget the reference, which is wrong.
                // Array::default(()) does not panic if T does not panic
                // if you restrict which types you use Array with, this should be fine.
                mem::forget(mem::replace(array, Array::default(()) ));
                Err(e).unwrap()
            }
        };
        mem::swap(array, &mut tmp);

        mem::forget(tmp);
    }
}
#18

Do we really need unsafe for this? How about:

fn inplace_reshape<T>(array: &mut ArrayD<T>, shape: Vec<usize>) -> Result<(), ShapeError> {
    let mut tmp = Array::from_vec(vec![]).into_dyn();
    mem::swap(array, &mut tmp);
    tmp = tmp.into_shape(shape)?;
    mem::swap(array, &mut tmp);
    Ok(())
}
2 Likes
#19

@KrishnaSannasi Thank you again.
@cuviper Yes, maybe this is even better, but my actual goal was to achieve the reshape inplace without the need for a temporary value.

#20

Array::from_vec is really cheap, so I would profile your code using @cuviper’s method before going for unsafe.

1 Like
#21

I’ll definitely do it. But I also thought it would be a good example to try it with unsafe :wink:

1 Like