VUnit/vunit

Indexing order for 3D `integer_array_t` doesn't make sense

bradleyharden opened this issue · 0 comments

The indexing order for the 3D version of integer_array_t doesn't make any sense. You can see the code for get here:

return get(arr.data, (y*arr.width + x)*arr.depth + z);

y is multiplied by both width and depth, x is only multiplied by depth, and z isn't multiplied by anything.

The indexing of get is consistent with set, which is why it hasn't been caught until now. The problem comes when you try to index after a reshape, which is how I discovered the problem. If you reshape a 3D array into a 2D array, you can't get results that make sense.

The "obvious" solution would be

< return get(arr.data, (y*arr.width + x)*arr.depth + z);
---
> return get(arr.data, (x*arr.width + y)*arr.depth + z);

But this solution would end up inconsistent with the 2D version of get:

return get(arr.data, y*arr.width + x);

I'm not really sure what to do here. It seems like the change that would be least likely to break existing code would be

return get(arr.data, (z*arr.depth + y)*arr.width + x);

since that is consistent with the existing indexing order for 2D arrays.

Has anyone ever decided whether integer_array_t stores data in row-major or column-major order? The indexing for 2D arrays suggests column-major, but that goes against C and Python. It seems like that is something that should be documented and guaranteed.