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.
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?
@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.
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.
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
}
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
}
}
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.
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.
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.
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);
}
}
@RustyYato 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.
Aside: I like how the into_shape function can fail without returning any array (fallible conversion functions should always (unless Copy) return the input wrapped in the error type), making even @cuviper's solution also lose the elements in case of an error despite the clean error handling...
Now, to the matter of the discussion, I'll just say that uninitialized memory is one of the most dangerous things to do in Rust (move semantics and destructors, invalid bit layouts, any detail can lead to UB); there are actually plans to deprecate mem::uninitialized(). Listen to @RustyYato's warnings and go with something along the lines of @cuviper's nice solution.
