ndarray icon indicating copy to clipboard operation
ndarray copied to clipboard

Construction from negative strides?

Open kngwyu opened this issue 5 years ago • 3 comments

Hi, thank you for this great library.

I encountered a problem when creating an ArrayView with a pointer and negative strides (see https://github.com/PyO3/rust-numpy/issues/151 for the full context). We can do this by converting &[isize] to &[usize], but this causes panic by a debug assertion when building without --release flag. So,

  • How about adding some unsafe API to do this? It would be like a public new_. I'm happy to create a PR if you're willing to.
  • Why are strides internally usize? It can be negative by slicing and I don't think it's reasonable to use usize.

kngwyu avatar Sep 19 '20 10:09 kngwyu

I started working on support for negative strides in the various constructors a while ago (branch with work-in-progress changes) but didn't get a chance to finish. Most of the necessary changes are in the internal functions which check the validity of the inputs. For the public API, the primary change will be modifying ShapeBuilder to allow isize strides.

You could finish up the necessary changes to ndarray to support negative strides in the constructors (or, I suppose, the subset of changes needed for just the unsafe view constructors). Or, as a temporary workaround, you could use a function which does the following:

  1. For each axis with a negative stride, offset the pointer by (axis_len - 1) * stride.
  2. Construct a view using the absolute values of the strides.
  3. For each axis that had a negative stride, call .invert_axis().

Why are strides internally usize?

They are represented as usize in the ArrayBase struct, but everywhere they're used, they're cast to isize. Representing them as usize is a hack to have a relatively simple design while dealing with limitations of the type system. With some of the upcoming Rust features (generic associated types in particular), it'll be possible to resolve this in a much cleaner way.

jturner314 avatar Sep 19 '20 22:09 jturner314

Thank you for the detailed explanation! I'll use a temporary workaround for now, but am willing to tackle the ShapeBuilder change if I could make time.

Representing them as usize is a hack to have a relatively simple design

That makes sense, though I hope some generics features would resolve this.

kngwyu avatar Sep 20 '20 03:09 kngwyu

Unchanged situation after the recent PRs including #948, but now it's explicitly documented that it's unsupported (for now).

bluss avatar Mar 23 '21 22:03 bluss