Updated Windows base Computations to be Safer

[LazaroHurtado]

Please refer to #1249 for context and discussions, specifically this comment.

The assertion verifying a window's size dimension or axis' stride is non-zero has been removed and outsourced to Slice, which will error when:

1. end > ax_desc.len, which can only happen when wsz == 0
2. step == 0, which can only happen when an axis' stride is zero

Thus, we maintain the same functionality as before

[nilgoyette]

Here we'll have panics (debug mode) due to wraparound immediately if the user passes in any array with inverted axis (negative stride)

I don't want to ask too much of a generous contributor, but if you can, could you please add a test that would have failed before the fix in this PR? (a test with negative strides)

[LazaroHurtado]

could you please add a test that would have failed before the fix in this PR? (a test with negative strides)

No worries! But I am not totally sure if this is possible, windows_with_stride requires stride to implement IntoDimension<Dim = D> which already restricts each axis to be non-negative (type Ix = usize;). If I'm overlooking something please let me know

[nilgoyette]

If it's impossible to pass an array with negative strides, then I don't understand the point of this PR.

Can't you call windows on a transposed matrix?

m.t().windows(...)

If I understand bluss comment correctly, it should create negative strides.

[LazaroHurtado]

Ah sorry there are two strides here, the parameter and the offset, I was thinking about the parameter. I believe there's already a windows test that creates a matrix with negative strides by inverting it here but I can create a new test for both windows and windows_with_stride that acts on a transposed matrix as you suggested

[bluss]

If I understand bluss comment correctly, it should create negative strides.

Transpose just changes order of axes, it doesn't modify their lengths or strides. "Axis 0 and 1 become axis 1 and 0".

Use instead ndarray's method .invert_axis(Axis(0)), that will reverse the matrix along that axis. (*)

Example on matrix with dimensions (n1, n2) and strides (s1, s2):

• (n1, n2) and (s1, s2) -- transpose -> (n2, n1) and (s2, s1)
• (n1, n2) and (s1, s2) -- invert axis 0 -> (n1, n2) and (-s1, s2)

(*) slicing with negative step is equivalent to using invert axis. As seen in that test code: array.slice(s![.., ..;-1])

[bluss]

I don't want to ask too much of a generous contributor, but if you can, could you please add a test that would have failed before the fix in this PR? (a test with negative strides)

I agree with this one.

@LazaroHurtado We need a negative axis input array. The window strides (your feature) still just needs to be anything that's not just (1, 1, 1), for example something with a stride of 2 on the same axis that is inverted. I agree no negative window strides should be possible to be used.

[LazaroHurtado]

Thank you for adding some more context! I included a test that asserts the proper strided (the parameter) windows are returned when a single and all axis are inverted. I also verified that this test failed before the work introduced in this PR.

[nilgoyette]

Excellent, thank you @LazaroHurtado for the test, and @bluss for the technical details. Now I feel much more confident merging this branch!

I tested with numpy and I get the same results.