Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement integer array indexing #651

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

s-ol
Copy link
Contributor

@s-ol s-ol commented Dec 12, 2023

Close #607.

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

TODO:

  • u/int index types
  • 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
Copy link
Contributor Author

s-ol commented Dec 12, 2023

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;
 }

@v923z
Copy link
Owner

v923z commented Dec 12, 2023

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

@v923z
Copy link
Owner

v923z commented Dec 12, 2023

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

@v923z
Copy link
Owner

v923z commented Dec 12, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST] add indexing via array
2 participants