[FEATURE REQUEST] Add internal function to create ndarray from byte array.

[kwagyeman]

Right now if I want to create an ndarray from a byte buffer I have to manually do it as there's no internal function for this:

ndarray = m_new_obj(ndarray_obj_t);
ndarray->base.type = &ulab_ndarray_type;
ndarray->dtype = dtype_code;
ndarray->boolean = NDARRAY_NUMERIC;
ndarray->ndim = ndim;
ndarray->len = len * channels;
ndarray->itemsize = dtype_size;
memcpy(ndarray->shape, shape, sizeof(shape));
memcpy(ndarray->strides, strides, sizeof(strides));
ndarray->array = bufinfo.buf;
ndarray->origin = bufinfo.buf;

I found this code by looking at what numpy/create.c does. It would be nice if there was a C function in ndarray.c/.h that initialized the array like how ndarray_new_dense_ndarray works but where you can just pass it a byte array.

[v923z]

Do I understand correctly that all you require is a function in C that basically does exactly what ndarray_new_dense_ndarray does, except that, as an additional argument takes the pointer to the buffer? Would it be OK, if we simply extended ndarray_new_dense_ndarray with a usually NULL argument, and when that's not NULL, then we simply assign the pointers?

ndarray->array = bufinfo.buf;
ndarray->origin = bufinfo.buf;

This would mean that we'd have to update all calls to ndarray_new_dense_ndarray, but the number of those calls is limited, and then we wouldn't duplicate any code.

[kwagyeman]

That would work. Ibrahim just pointed out that with us manually assigning the class like we are currently doing, it could cause maintenance issues.

[kwagyeman]

See the code here: https://github.com/openmv/openmv/pull/2293/files#diff-8aefd1bad3f1feba530f85001ac7031dcd760183316b08412dfc7012d5c238f7R794

Note, all OpenMV Cam models are getting 4D ndarray support and TensorFlow support with all ops enabled. See here for what this will mean: https://docs.google.com/presentation/d/1GWdpCt-iQbVJC6vSOqaNAUnuoqbA7GUdXr7nPqzcASI/edit#slide=id.g2785cd2492b_0_51

We are embracing ulab and ndarrys now!

[v923z]

I've just checked, ndarray_new_dense_ndarray is called 244 times at various places of the code, so what I would do is define a new function, ndarray_from_buffer, which would call ndarray_new_dense_ndarray, and then simply overwrite the pointer:

ndarray_obj_t *ndarray_from_buffer(uint8_t ndim, size_t *shape, uint8_t dtype, uint8 *buffer) {
    ndarray_obj_t *ndarray = ndarray_new_dense_ndarray(ndim, shape, dtype);
    ndarray->array = buffer;
    ndarray->origin = buffer;
    return ndarray;
}

Would this work in your case? I don't know if you want to implement error checking, and how. Somehow you've got to prevent the pointer from going over the buffer's boundary.

[v923z]

Oh, wait! This would reserve space for the array, and that's not what you want. https://github.com/v923z/micropython-ulab/blob/master/code/utils/utils.c already implements something very similar, except, the output is one-dimensional.

[v923z]

I can hash out something tomorrow night, if that's not too late.

[kwagyeman]

Uh, yeah, wherever, we'll release with what you have now and then update when we pull whatever the latest lab is.

[v923z]

I think I have a firm idea now, it's really not that hard. Just tell me what your arguments are! You must have at least uint8_t ndim, size_t *shape, uint8_t dtype, and perhaps, int32_t *strides.

[kwagyeman]

The same arguments as ndarray_new_dense_ndarray but with the ability to specify where the buffer is versus it being alloced.

[v923z]

Can you, please, check if you can work with this? https://github.com/v923z/micropython-ulab/tree/buffer.

Also note that this adds approx. 2 kB to the unix port. I don't know how representative that is, but if similar changes in size are recorded on other ports, then I would roll this back to the current master, and add the full implementation of the function. I believe the increase in size is not because of the body of the new function (that is almost trivial), but the extra NULL argument that must be carried all over the place.

Let me know how you want to proceed!

[kwagyeman]

2KB is a lot of space. We are actually flash limited. Maybe it would be better just to make a function that does the array initialization like I was doing, and put a method for it in the header file? Then it would have zero cost except when used.

[v923z]

OK, no problem. You still got to do some sanity checks, it's not only the initialisation.

[v923z]

This is the size of the unix port in bytes:

65c941a [master]: 5938992
68017a2 [buffer]: 5941576
d4373ab [buffer]: 5940904

[v923z]

So, it seems that the exceptions eat a lot of firmware space: if I remove the three of them from the function in 68017a2, then the size is dropped to 5939776. This is something like 800 bytes, compared to 65c941a.

[kwagyeman]

Mmm, I see you are trying to make this generic for all invocations and usage. Update the library as you like.

I just need it to be used to create a dense ndarray from a buffer versus new()

[v923z]

An alternative would be to re-use

micropython-ulab/code/numpy/create.c

Line 808 in 6fb60ef

mp_obj_t create_frombuffer(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {

, and then simply reshape the results at the end with

micropython-ulab/code/ndarray.c

Line 2008 in 6fb60ef

mp_obj_t ndarray_reshape_core(mp_obj_t oin, mp_obj_t _shape, bool inplace) {

.

[v923z]

Mmm, I see you are trying to make this generic for all invocations and usage. Update the library as you like.

I just need it to be used to create a dense ndarray from a buffer versus new()

If you have other mechanisms to check for errors (you certainly don't want to move the array pointer beyond the boundary of the buffer), then it might be easiest to implement the function in openmv, because then you can strip it down to the bare minimum.

[kwagyeman]

Yeah, I don't want to call a micropython method from C though for this if I can just build the object directly.

[v923z]

OK, so what do you want me to do?

[kwagyeman]

I guess the branch you built is fine. 800 bytes is not a problem.

[v923z]

It's 800 bytes without the exceptions. What do you want to do, if the requested size is beyond the buffer's bounds?

[kwagyeman]

So, I have already checked these things when calling the C API. You'd expect all error checks to have already been made... right?

[v923z]

If you intend to call this function at multiple places, then it would make sense to move the error checking into this function. So, if you'd want to have a trimmed-down function, then it really makes sense to implement this in openmv.

If this is something that only openmv uses, then I don't actually see any point in moving it to ulab. Even from the maintenance perspective, that would be unwise.

[kwagyeman]

Give me what you think is best. When the code is compiled with -O2 for arm is probably a lot smaller than 800 bytes in size usage.

[v923z]

OK, so 2b274f1 reduces the firmware size by 272 bytes compared to 65c941a. (The saving comes from

micropython-ulab/code/numpy/create.c

Line 808 in 2b274f1

mp_obj_t create_frombuffer(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {

, where the ndarray_new_ndarray function can be called instead of having to write out the ndarray's header's assignment.)

If you know what you're doing, then you can simply use

micropython-ulab/code/ndarray.c

Line 512 in 2b274f1

ndarray_obj_t *ndarray_new_ndarray(uint8_t ndim, size_t *shape, int32_t *strides, uint8_t dtype, uint8_t *buffer) {

, and pass the pointer to the buffer as the last argument.

[v923z]

Just to avoid any confusion, 65c941a is the current master, so with this modification, we would reduce the current firmware size by 272 bytes. I don't think you're going to get a better bargain.

[kwagyeman]

Sounds good!

[v923z]

Implmented in #677.