| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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) |
Sorry, something went wrong.
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 |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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 |
Sorry, something went wrong.
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):
(*) slicing with negative step is equivalent to using invert axis. As seen in that test code: array.slice(s![.., ..;-1]) |
Sorry, something went wrong.
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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:
Thus, we maintain the same functionality as before