3x1x3 interleaved layout returns y().stride() of 1 instead of 9 about array HOT 4 CLOSED

I think this is the same problem as here: https://github.com/dsharlet/array/blob/master/test/shape.cpp#L184

This has come up before in other contexts too. The problem is that when the extent is 1, there's no real constraint on the stride, so the automatic stride algorithm just gives it something that seems reasonable to it. I think this is hard to fix without baking some crappy heuristic into the algorithm. @fbleibel-g might have more information about those other contexts he can share with you.

from array.

Hi Dillon,

I tried adding a few lines to solve this issue, and it seemed to work. In 'is_stride_ok()' in array.h, I did the following (inserted the middle 'if()' block):

if (is_unresolved(dim.stride())) {
  // If the dimension has an unknown stride, it's OK, we're
  // resolving the current dim first.
  return true;
}
if (extent == 1 && abs(stride) == abs(dim.stride()) && dim.extent() > 1) {
  // Special-cases "extent == 1" to avoid unexpectedly giving a stride of 1 for
  // outer dimensions; e.g. the y-stride for single-row interleaved images, or
  // the plane-stride for single-plane Planar images.
  return false;
}
if (dim.extent() * abs(dim.stride()) <= stride) {
  // The dim is completely inside the proposed stride.
  return true;
}

This "fixes" the cases mentioned. It also enables the commented-out test you mentioned above. Curious what you think? If it looks ok, I can submit it as a PR, or feel free to incorporate it.

Thanks,
Ben

from array.

Hmm, that is a lot less crufty than I thought would be necessary.

I played around with this a bit and added some more tests to see what it does in these cases:

  // Stride one dimensions mixed in with extent 1 dimensions.
  check_resolved_strides<3>({1, 1, {0, 2, 1}}, {2, 2, 1});
  check_resolved_strides<3>({1, {0, 2, 1}, 1}, {2, 1, 2});
  check_resolved_strides<3>({1, 10, {0, 2, 1}}, {2, 2, 1});
  check_resolved_strides<3>({5, {0, 2, 1}, 1}, {2, 1, 10});

It does produce reasonable results in these cases.

I think you should send it as a PR (with the extra tests above), this is a really nice improvement.

from array.

Thanks Dillon, I've sent a PR with my changes and your added tests. Let me know if it needs a CLA.

The only non-obvious side effect I've encountered from this change is that it is no longer possible to implicitly cast between interleaved and planar single-channel images, since the channel strides now differ. Probably this is a good thing; it's most likely a bug if anyone's code was trying to do that. But the assertion it triggers could trivially be modified to still allow it, if there were any reason to.

from array.