micropython-ulab icon indicating copy to clipboard operation
micropython-ulab copied to clipboard

Implement integer array indexing

Open s-ol opened this issue 1 year ago • 4 comments

Close #607.

This implements integer array indexing with the following limitations as of now:

TODO:

  • [x] u/int index types
  • [x] multi-dimensional index arrays
  • [ ] negative indices
  • [ ] index array range-checking
  • [ ] slice-assignment

non-goals for this PR:

  • support N-dim target types
  • support float index types (debatable)

s-ol avatar Dec 12 '23 18:12 s-ol

Note: when compiling this from latest CircuitPython master, I need the following patch to avoid a GCC false positive:

diff --git a/code/ndarray.c b/code/ndarray.c
index c41c01a..46c84b3 100644
--- a/code/ndarray.c
+++ b/code/ndarray.c
@@ -496,7 +496,10 @@ bool ndarray_is_dense(ndarray_obj_t *ndarray) {
     // the array should be dense, if the very first stride can be calculated from shape
     int32_t stride = ndarray->itemsize;
     for(uint8_t i = ULAB_MAX_DIMS - 1; i > ULAB_MAX_DIMS-ndarray->ndim; i--) {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Warray-bounds"
         stride *= ndarray->shape[i];
+#pragma GCC diagnostic pop
     }
     return stride == ndarray->strides[ULAB_MAX_DIMS-ndarray->ndim] ? true : false;
 }

s-ol avatar Dec 12 '23 18:12 s-ol

You seem to bail out, when the array is not dense, but I think your INDEX_LOOP actually handles the general case.

v923z avatar Dec 12 '23 19:12 v923z

I think it's OK to not deal with float indices. Also, make this optional by defining a variable in ulab.h!

v923z avatar Dec 12 '23 19:12 v923z

If I look at your INDEX_LOOP, I have the feeling that you can save half of the space, if you copy the bytes corresponding to the contents of the RAM instead of retrieving the value. You could treat int8/uint8, and int16/uint16 on the same footing, because only the size matter, but not the signedness of the value.

v923z avatar Dec 12 '23 19:12 v923z