PyArray::to_owned_array() panics for reversed array

[hombit]

use numpy;
use pyo3::types::IntoPyDict;

fn main() {
    let gil = pyo3::Python::acquire_gil();
    let py = gil.python();
    let locals = [("np", numpy::get_array_module(py).unwrap())].into_py_dict(py);
    let not_contiguous: &numpy::PyArray1<f32> = py
        .eval("np.zeros(2)[::-1]", Some(locals), None)
        .unwrap()
        .downcast()
        .unwrap();
    let _owned = not_contiguous.to_owned_array();
}

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: ShapeError/Overflow: arithmetic overflow', /Users/hombit/.cargo/registry/src/github.com.-1ecc6299db9ec823/ndarray-0.13.1/src/impl_raw_views.rs:75:13
stack backtrace:
   0: <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt
   1: core::fmt::write
   2: std::io::Write::write_fmt
   3: std::panicking::default_hook::{{closure}}
   4: std::panicking::default_hook
   5: std::panicking::rust_panic_with_hook
   6: rust_begin_unwind
   7: core::panicking::panic_fmt
   8: core::option::expect_none_failed
   9: core::result::Result<T,E>::unwrap
  10: ndarray::impl_raw_views::<impl ndarray::ArrayBase<ndarray::RawViewRepr<*const A>,D>>::from_shape_ptr
  11: ndarray::impl_views::constructors::<impl ndarray::ArrayBase<ndarray::ViewRepr<&A>,D>>::from_shape_ptr
  12: numpy::array::PyArray<T,D>::as_array
  13: numpy::array::PyArray<T,D>::to_owned_array
  14: pyo3_panics::main
  15: std::rt::lang_start::{{closure}}
  16: std::rt::lang_start_internal
  17: std::rt::lang_start
  18: main

[hombit]

It looks like the problem is in the last line of strides_usize() where a negative stride is converted to usize

rust-numpy/src/array.rs

Lines 300 to 307 in 8c8c927

	fn strides_usize(&self) -> &[usize] {
	let n = self.ndim();
	let ptr = self.as_array_ptr();
	unsafe {
	let p = (*ptr).strides;
	slice::from_raw_parts(p as *const _, n)
	}
	}

[kngwyu]

Thanks, I tested it now, and your observation looks correct 👍

[kngwyu]

Returning an error when the stride has negative integers would work, but seems like a hack.
Please let me know if you come up with a better solution.

[hombit]

I don't know how to make it work with ndarray, because I cannot find the right constructor, but its docs say it supports negative strides. A solution could be a creation of an ndarray::Array from the data and setting strides after

[kngwyu]

but its docs say it supports negative strides.

But I think its internal strides representation uses usize...

[hombit]

It looks like ArrayView.strides() returns &[isize]. Can it be a solution to create an ArrawView from absolute values of strides and then slice it with something like s![..; -1]?

use ndarray::*;

fn main() {
    const N: usize = 16;
    let data: Vec<_> = (0..N).map(|i| i as f64).collect();
    let a = ArrayView::from_shape(N, &data).unwrap();
    let slice = a.slice(s![..; -1]);
    println!("{:?}", slice);
}

[15.0, 14.0, 13.0, 12.0, 11.0, 10.0, 9.0, 8.0, 7.0, 6.0, 5.0, 4.0, 3.0, 2.0, 1.0, 0.0], shape=[16], strides=[-1], layout=Custom (0x0), const ndim=1

[kngwyu]

Well but it actually holds strides as unsigned. See https://docs.rs/ndarray/0.13.1/src/ndarray/impl_methods.rs.html#119-123.

So somehow we need to reverse the stride to allow negative stride, as s! does, but I still don't understand the logic.

[hombit]

Oh, I see. Thank you for your commitment to solve the problem!

[kngwyu]

So... I noticed that this is due to the debug assertion and does not happen with --release, since ndarray internally uses usize to represent negative strides.
E.g., I confirmed that this works:

    let gil = pyo3::Python::acquire_gil();
    let py = gil.python();
    let arr = array![[2, 3], [4, 5u32]];
    let pyarr = arr.to_pyarray(py);
    let negstr_pyarr: &numpy::PyArray2<u32> = py
        .eval("a[::-1]", Some([("a", pyarr)].into_py_dict(py)), None)
        .unwrap()
        .downcast()
        .unwrap();
    assert_eq!(negstr_pyarr.to_owned_array(), arr.slice(s![..;-1, ..]));

I'm considering opening an upstream issue but not sure.

[kngwyu]

I made #156 and will merge it before the next release (soon).

[hombit]

Thank you so much!