building in CircuitPython

[GathererA]

This is amazing! I've been trying to build ulab in CircuitPython, which I should be able to do as CP is a fork of micropython, but I've been having a lot of trouble. I'm seeing a lot of issues with shadowed declarations and comparison between signed and unsigned integers. When I attempt to make the board in the appropriate port using:

make BOARD=pybadge USER_C_MODULES=../../../ulab all

I get the following error:

Including User C Module from ../../../ulab/code QSTR updated FREEZE ../../frozen/circuitpython-stage/pybadge ../../../ulab/code/ndarray.c: In function 'ndarray_print_row': ../../../ulab/code/ndarray.c:81:20: error: declaration of 'i' shadows a previous local [-Werror=shadow] for(size_t i=1; i<3; i++) { ^ ../../../ulab/code/ndarray.c:66:12: note: shadowed declaration is here size_t i; ^ ../../../ulab/code/ndarray.c: In function 'ndarray_make_new': ../../../ulab/code/ndarray.c:168:30: error: passing argument 2 of 'mp_arg_check_num' makes pointer from integer without a cast [-Werror=int-conversion] mp_arg_check_num(n_args, n_kw, 1, 2, true); ^~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:80:6: note: expected 'mp_map_t {aka struct _mp_map_t }' but argument is of type 'size_t {aka unsigned int}' void mp_arg_check_num(size_t n_args, mp_map_t kw_args, size_t n_args_min, size_t n_args_max, bool takes_kw); ^~~~ ../../../ulab/code/ndarray.c:176:29: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("first argument must be an iterable"); ^~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c:191:25: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] if(len2 != MP_OBJ_SMALL_INT_VALUE(len_in)) { ^~ ../../../ulab/code/ndarray.c:192:41: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("iterables are not of the same length"); ^~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c: In function 'true_length': ../../../ulab/code/ndarray.c:234:13: error: implicit declaration of function 'mp_obj_is_type'; did you mean 'mp_obj_is_true'? [-Werror=implicit-function-declaration] if(!mp_obj_is_type(item, &mp_type_bool)) { ^~~~~~ mp_obj_is_true ../../../ulab/code/ndarray.c:234:13: error: nested extern declaration of 'mp_obj_is_type' [-Werror=nested-externs] ../../../ulab/code/ndarray.c: In function 'generate_slice': ../../../ulab/code/ndarray.c:251:15: error: implicit declaration of function 'mp_obj_is_int'; did you mean 'mp_obj_list_init'? [-Werror=implicit-function-declaration] } else if(mp_obj_is_int(index)) { ^~~~~ mp_obj_list_init ../../../ulab/code/ndarray.c:251:15: error: nested extern declaration of 'mp_obj_is_int' [-Werror=nested-externs] ../../../ulab/code/ndarray.c:256:20: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare] if((_index >= n) || (_index < 0)) { ^~ ../../../ulab/code/ndarray.c:257:47: error: passing argument 2 of 'mp_raise_msg' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_msg(&mp_type_IndexError, "index is out of bounds"); ^~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:151:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_msg(const mp_obj_type_t exc_type, const compressed_string_t msg); ^~~~ ../../../ulab/code/ndarray.c:263:43: error: passing argument 2 of 'mp_raise_msg' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_msg(&mp_type_IndexError, "indices must be integers, slices, or Boolean lists"); ^~~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:151:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_msg(const mp_obj_type_t exc_type, const compressed_string_t msg); ^~~~ ../../../ulab/code/ndarray.c: In function 'insert_slice_list': ../../../ulab/code/ndarray.c:293:33: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("could not broadast input array from shape"); ^~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c:355:27: error: declaration of 'cindex' shadows a previous local [-Werror=shadow] size_t j = 0, cindex = 0; ^~ ../../../ulab/code/ndarray.c:296:12: note: shadowed declaration is here size_t cindex, rindex; ^~ ../../../ulab/code/ndarray.c: In function 'iterate_slice_list': ../../../ulab/code/ndarray.c:380:43: error: passing argument 2 of 'mp_raise_msg' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_msg(&mp_type_IndexError, "empty index range"); ^~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:151:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_msg(const mp_obj_type_t exc_type, const compressed_string_t msg); ^~~~ ../../../ulab/code/ndarray.c:441:27: error: declaration of 'cindex' shadows a previous local [-Werror=shadow] size_t j = 0, cindex = 0; ^~ ../../../ulab/code/ndarray.c:390:12: note: shadowed declaration is here size_t cindex, rindex; ^~ ../../../ulab/code/ndarray.c: In function 'ndarray_get_slice': ../../../ulab/code/ndarray.c:498:47: error: passing argument 2 of 'mp_raise_msg' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_msg(&mp_type_IndexError, "too many indices"); ^~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:151:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_msg(const mp_obj_type_t exc_type, const compressed_string_t msg); ^~~~ ../../../ulab/code/ndarray.c:506:51: error: passing argument 2 of 'mp_raise_msg' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_msg(&mp_type_IndexError, "indices must be integers, slices, or Boolean lists"); ^~~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:151:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_msg(const mp_obj_type_t exc_type, const compressed_string_t msg); ^~~~ ../../../ulab/code/ndarray.c: In function 'ndarray_subscr': ../../../ulab/code/ndarray.c:547:33: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("right hand side must be an ndarray, or a scalar"); ^~~~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c: In function 'ndarray_flatten': ../../../ulab/code/ndarray.c:658:29: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("flattening order must be either 'C', or 'F'"); ^~~~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c: In function 'ndarray_binary_op': ../../../ulab/code/ndarray.c:723:33: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("operands could not be broadcast together"); ^~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c:826:40: error: passing argument 1 of 'mp_raise_TypeError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_TypeError("wrong input type"); ^~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:155:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_TypeError(const compressed_string_t msg); ^~~~~~ ../../../ulab/code/ndarray.c:834:28: error: passing argument 1 of 'mp_raise_TypeError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_TypeError("wrong operand type on the right hand side"); ^~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:155:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_TypeError(const compressed_string_t msg); ^~~~~~ ../../../ulab/code/ndarray.c: In function 'ndarray_unary_op': ../../../ulab/code/ndarray.c:852:37: error: passing argument 1 of 'mp_raise_ValueError' from incompatible pointer type [-Werror=incompatible-pointer-types] mp_raise_ValueError("operation is not supported for given type"); ^~~~~~~~~~~ In file included from ../../../ulab/code/ndarray.c:15:0: ../../py/runtime.h:153:15: note: expected 'const compressed_string_t {aka const struct }' but argument is of type 'const char ' NORETURN void mp_raise_ValueError(const compressed_string_t msg); ^~~~~~~ ../../../ulab/code/ndarray.c:865:26: error: declaration of 'array' shadows a previous local [-Werror=shadow] uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:868:25: error: declaration of 'array' shadows a previous local [-Werror=shadow] int8_t array = (int8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:871:27: error: declaration of 'array' shadows a previous local [-Werror=shadow] uint16_t array = (uint16_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:874:26: error: declaration of 'array' shadows a previous local [-Werror=shadow] int16_t array = (int16_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:877:29: error: declaration of 'array' shadows a previous local [-Werror=shadow] mp_float_t array = (mp_float_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:892:25: error: declaration of 'array' shadows a previous local [-Werror=shadow] int8_t array = (int8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:897:26: error: declaration of 'array' shadows a previous local [-Werror=shadow] int16_t array = (int16_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:902:29: error: declaration of 'array' shadows a previous local [-Werror=shadow] mp_float_t array = (mp_float_t )ndarray->array->items; ^~~~~ ../../../ulab/code/ndarray.c:857:22: note: shadowed declaration is here uint8_t array = (uint8_t )ndarray->array->items; ^~~~~ cc1: all warnings being treated as errors make: *** [build-pybadge/code/ndarray.o] Error 1

[GathererA]

also fails when attempting to use the GNU Arm Embedded Toolchain: make BOARD=pybadge CROSS_COMPILE=../../gcc-arm-none-eabi-9-2019-q4-major/bin/arm-none-eabi- USER_C_MODULES=../../../ulab all

[GathererA]

the circuitpython/py files are significantly different than the micropython/py files, I'm wondering if that could be the original issue and if there's any solution

[dhalbert]

CircuitPython has translated messages, so change all the mp_raise...("some message") to mp_raise(translate("some message")). Also we use -Wall and catch all warnings, so the warnings are probably things that could be fixed to not raise a warning.

And, there are some internal API changes. So I think the best thing would be to fork and make some changes.

We (Adafruit) have considerable interest in getting ulab to work eventually -- it's on our agenda, but we have some shorter-term things to finish first.

[GathererA]

Thanks! Working on changing the internal API stuff around to work out. Quick question, there are a lot of functions, specifically MP_OBJ_FUN_MAKE_SIG that are in micropython but not circuitpython. Where can I find these functions in the libraries? I can't seem to find them in the circuitpython/py or micropython/py libraries.

[dhalbert]

That's a macro. grep -r or ag is your friend. Note that CircuitPython is based off of micropython 1.9.4, so nearly everything that has been added since then we don't have (we did cherry-pick a few fixes). We will merge from upstream eventually but it's a big job.

[v923z]

CircuitPython has translated messages, so change all the mp_raise...("some message") to mp_raise(translate("some message")). Also we use -Wall and catch all warnings, so the warnings are probably things that could be fixed to not raise a warning.

And, there are some internal API changes. So I think the best thing would be to fork and make some changes.

We (Adafruit) have considerable interest in getting ulab to work eventually -- it's on our agenda, but we have some shorter-term things to finish first.

If the micropython/circuitpython differences are reasonable (meaning that compilation can be facilitated by means of macros), I could insert those into the code. I just don't have experience with circuitpython, so I don't even know, where to look. Could we make this work by doing something like this:

#IF MICROPYTHON
   mp_raise_ValueError("bad luck");
#ELSEIF CIRCUITPYTHON
   some circuitpython machinery
#ENDIF

Frankly, I would prefer, if we could find a solution that works on all platforms, as opposed to telling the user "here are some hints, go, fix it yourself".

As for forking, sure. The snag is, I am working on an update of ulab, and if you want to include the new features, you have to update your fork accordingly. That will become intractable very quickly.

[GathererA]

I believe I've made the ulab/code mostly compatible with CircuitPython through only editing the ulab files rather than also editing the CP files. Now I'm getting this error and I'm not sure where it's coming from:

(base) MacBook-Pro-39:atmel-samd agatherer$ make BOARD=pybadge USER_C_MODULES=../../../ulab all
Use make V=1, make V=2 or set BUILD_VERBOSE similarly in your environment to increase build verbosity.
Including User C Module from ../../../ulab/code
QSTR not updated
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_acosh':
<artificial>:(.text.vectorise_acosh+0x8): undefined reference to `acoshf'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_asinh':
<artificial>:(.text.vectorise_asinh+0x8): undefined reference to `asinhf'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_atanh':
<artificial>:(.text.vectorise_atanh+0x8): undefined reference to `atanhf'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_erf':
<artificial>:(.text.vectorise_erf+0x8): undefined reference to `erff'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_erfc':
<artificial>:(.text.vectorise_erfc+0x8): undefined reference to `erfcf'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_gamma':
<artificial>:(.text.vectorise_gamma+0x8): undefined reference to `tgammaf'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_lgamma':
<artificial>:(.text.vectorise_lgamma+0x8): undefined reference to `lgammaf'
/var/folders/qv/87ltljgj73753mc00909bvm00000gn/T//ccga3dDh.lto.o: In function `vectorise_log2':
<artificial>:(.text.vectorise_log2+0x8): undefined reference to `log2f'
collect2: error: ld returned 1 exit status
make: *** [build-pybadge/firmware.elf] Error 1

[GathererA]

Navigating to the folders, the files in question are gone, so I suspect they're temporary files generated by my Mac OSX 10.5

[GathererA]

Could it be something to do with the different version of MICROPY that CircuitPython uses? The problem persists when I manually change the MICROPY_VERSION_MAJOR, MICROPY_VERSION_MINOR, and MICROPY_VERSION_MICRO to match what worked for when I build ulab in MicroPython.

[v923z]

Navigating to the folders, the files in question are gone, so I suspect they're temporary files generated by my Mac OSX 10.5

I will try to compile circuitpython on a linux machine, perhaps, we can learn something from that. The errors that you quoted above seem to be related to math.h. We could begin with a stripped-down version of ulab. You can remove vectorise.c, fourier.c, numerical.c, and linear.c from your make file, and at the same time, you should also remove all functions in ulab.c that are based on these files. I believe, at that point you should be able to compile the module.

Alternatively, you could grab something simple from https://micropython-usermod.readthedocs.io/, and see how far you get.

It is a bit unfortunate that circuitpython elected to diverge from micropython at the software level. I don't use anything in ulab that is related to hardware, so, in my opinion, the module should just work. I really don't see, how they are going to manage this in the future, and I feel that this is a bit of waste of resources.

[v923z]

CircuitPython has translated messages, so change all the mp_raise...("some message") to mp_raise(translate("some message")).

Does that mean that there is a single exception type in circuitpython? I am sure you have your reasons for modifying micropython, but exceptions are really a core concept of python, and I just can't imagine that yanking that can be justified. It is not an implementation detail at the C level, it concerns the python language itself.

I would be more than happy to modify ulab in a way that makes it compatible with circuitpython, but then we have to agree on some common ground, and I don't think that changes to python are negotiable.

[GathererA]

The board will compile if I use Linux rather than MacOS but has warnings that I've attached below. When I load the .uf2 file, though, and attempt to import ulab through the code.py file, then it still doesn't recognize ulab.

make BOARD=pybadge USER_C_MODULES=../../../ulab all Use make V=1, make V=2 or set BUILD_VERBOSE similarly in your environment to increase build verbosity. QSTR updated FREEZE ../../frozen/circuitpython-stage/pybadge ../../shared-bindings/fontio/Glyph.h:32:39: warning: size of 'fontio_glyph_type' differ from the size of original declaration [-Wlto-type-mismatch] extern const mp_obj_namedtuple_type_t fontio_glyph_type; ^ ../../shared-bindings/fontio/Glyph.c:49:32: note: 'fontio_glyph_type' was previously declared here const mp_obj_namedtuple_type_t fontio_glyph_type = { ^ ../../shared-bindings/usb_hid/init.h:33:23: warning: size of 'common_hal_usb_hid_devices' differ from the size of original declaration [-Wlto-type-mismatch] extern mp_obj_tuple_t common_hal_usb_hid_devices; ^ build-pybadge/autogen_usb_descriptor.c:157:16: note: 'common_hal_usb_hid_devices' was previously declared here mp_obj_tuple_t common_hal_usb_hid_devices = { ^ build-pybadge/frozen_mpy.c:141:26: warning: size of 'mp_qstr_const_pool' differ from the size of original declaration [-Wlto-type-mismatch] extern const qstr_pool_t mp_qstr_const_pool; ^ ../../py/qstr.c:99:19: note: 'mp_qstr_const_pool' was previously declared here const qstr_pool_t mp_qstr_const_pool = { ^ ../../py/qstr.c:116:26: warning: size of 'mp_qstr_frozen_const_pool' differ from the size of original declaration [-Wlto-type-mismatch] extern const qstr_pool_t MICROPY_QSTR_EXTRA_POOL; ^ build-pybadge/frozen_mpy.c:142:19: note: 'mp_qstr_frozen_const_pool' was previously declared here const qstr_pool_t mp_qstr_frozen_const_pool = { ^

177872 bytes free in flash firmware space out of 499712 bytes (488.0kB). 164552 bytes free in ram for stack and heap out of 196608 bytes (192.0kB).

Converting to uf2, output size: 644096, start address: 0x4000 Wrote 644096 bytes to build-pybadge/firmware.uf2.

[GathererA]

In retrospect, I'll probably have to put a specialized version of ulab in circuitpython/shared-modules as well as circuitpython/shared-bindings rather than including it in the command line ("make BOARD=pybadge USER_C_MODULES=../../../ulab all")

[v923z]

Thanks for looking into this. A couple of comments:

1. It seems to me that exceptions are raised in circuitpython as

   mp_raise_NotImplementedError(translate("FFT is defined for ndarrays only"));

   so, I believe, we could get away with defining translate as a do-nothing macro in ulab as

#ifdef MICROPYTHON
#translate(x) x
#endif

or something similar, and define the argument on the command line, when compiling for micropython.

2. I noticed that you called the absolute value function at a couple of places, where it was not necessary. I have commented those lines. Let me know if they must stay. Or is this the comment on comparing signed/unsigned integers?

3. mp_obj_is_type vs. MP_OBJ_IS_TYPE: was mp_obj_is_type function removed in circuitpython? The macro would compile into a larger firmware, I guess.

4. The call signature of ndarray_make_new is different in micropython. I would suggest that for that single case, we just bite the bullet, and have a macro switch.

5. In a couple of cases, you declare indices and other variables quite early in a function, so as to make them have a relatively large scope. Is that really necessary? First, you might not even need those variables, if, for whatever reason, the function is terminated at an early stage. Second, it is harder to keep track of the variable's scope.

In retrospect, I'll probably have to put a specialized version of ulab in circuitpython/shared-modules as well as circuitpython/shared-bindings rather than including it in the command line ("make BOARD=pybadge USER_C_MODULES=../../../ulab all")

What is required for compiling user modules in circuitpython? I have the feeling that you have sorted out most issues, so we should just mount an effort, and do the home stretch.

I assume, you haven't checked, whether the code still compiles for micropython? Should I do that? Before merging everything, we could work on the circuitpython branch. I would merge your changes into that first.

[dhalbert]

CircuitPython has translated messages, so change all the mp_raise...("some message") to mp_raise(translate("some message")).

Does that mean that there is a single exception type in circuitpython? I am sure you have your reasons for modifying micropython, but exceptions are really a core concept of python, and I just can't imagine that yanking that can be justified. It is not an implementation detail at the C level, it concerns the python language itself.

No, sorry, that was a typo, I meant mp_raise...(translate("some message")). The arguments just take a translate() string instead of a bare string.

[dhalbert]

In retrospect, I'll probably have to put a specialized version of ulab in circuitpython/shared-modules as well as circuitpython/shared-bindings rather than including it in the command line ("make BOARD=pybadge USER_C_MODULES=../../../ulab all")

What is required for compiling user modules in circuitpython? I have the feeling that you have sorted out most issues, so we should just mount an effort, and do the home stretch.

Just for information for others, CircuitPython refactored how native modules are coded. The Python side of an API is in shared-bindings/, and the lower-level implementations are in shared-modules/ (for port-independent parts of the code), or in port/<some_port>/common_hal/ for port-specific code. There are modules that have some port-independent code and some port-dependent code.

We did this split because we try to be very uniform across ports, rather than having port-specific native modules.

[v923z]

CircuitPython has translated messages, so change all the mp_raise...("some message") to mp_raise(translate("some message")).

Does that mean that there is a single exception type in circuitpython? I am sure you have your reasons for modifying micropython, but exceptions are really a core concept of python, and I just can't imagine that yanking that can be justified. It is not an implementation detail at the C level, it concerns the python language itself.

No, sorry, that was a typo, I meant mp_raise...(translate("some message")). The arguments just take a translate() string instead of a bare string.

OK, thanks for the clarification! Then the situation is not so bad.

[GathererA]

@dhalbert I'm having an issue where I can load the module (or a part of it that I've started testing with) into circuitpython just fine, but when I ask the REPL or the code.py to print the array, the board crashes. Are there any other mp related commands I'll need to change (as I changed mp_raise)? The print commands use mp_obj_print_helper and mp_print_str.

[GathererA]

adding translate doesn't seem to help in this case

[v923z]

The print commands use mp_obj_print_helper and mp_print_str.

That's a bit odd, because those two are declared in obj.h, in both circuitpython, and micropython.

What happens, when you compile for the unix port?

[dhalbert]

Are there any other mp related commands I'll need to change (as I changed mp_raise)? The print commands use mp_obj_print_helper and mp_print_str.

For most things, it wouldn't even compile without changing the calls, as you've done with translate(). The print commands should not really be different -- we have not made changes there. At this point I would take a look with a debugger. Do you have a J-Link or similar? I'd set a breakpoint on HardFault_Handler, and perhaps further back in the exception-raising code, if that's what's causing the error.

I just want to say again that we are quite interested in this port, but don't have the time at the moment to work on it ourselves for at least a few weeks. I'm sorry I can't try out your build myself right now.

[v923z]

Are there any other mp related commands I'll need to change (as I changed mp_raise)? The print commands use mp_obj_print_helper and mp_print_str.

For most things, it wouldn't even compile without changing the calls, as you've done with translate(). The print commands should not really be different -- we have not made changes there. At this point I would take a look with a debugger. Do you have a J-Link or similar? I'd set a breakpoint on HardFault_Handler, and perhaps further back in the exception-raising code, if that's what's causing the error.

I would really like to propose that we try to fix the issues on something much smaller. https://github.com/v923z/micropython-usermod/tree/master/snippets/simpleclass/simpleclass.c has everything that is related to printing, and is two orders of magnitude smaller than ulab. We should see, whether we can compile simpleclass.c., and if we fail, how can we modify it in a way that still keeps it in synch with micropython. The full section of the programming manual is in https://micropython-usermod.readthedocs.io/en/latest/usermods_08.html .

@dhalbert : is there a circuitpython programming manual somewhere, so that I could lend @GathererA a hand with this?

[dhalbert]

@dhalbert : is there a circuitpython programming manual somewhere, so that I could lend @GathererA a hand with this?

There is not really, any more than MicroPython has a manual. There is an older writeup here https://circuitpython.readthedocs.io/en/latest/docs/common_hal.html of how we structure the source tree. We in general have changed very little about the core interpreter and the Python virtual machine. The major changes we have made in those areas:

• A few changes to support better native-superclass->Python-subclass subtyping, causing changes in the signatures of some common calls (e.g., _make_new).
• A few more helper routines for common exceptions (mp_raise_<something>).
• Translations.
• Division of the heap into long- and short-lived objects to reduce fragmentation.
• Refactoring of native modules into shared-bindings/ (Python API) part and the shared-module/ and ports/*/common-hal/ (port-non-specfic and port-specific parts), as I mentioned earlier.
• A lot of refactoring of the build process to make it more parameterized and more applicable across multiple ports.

The other changes are all non-core-language, having to do with our own choice of the native module API's (busio.I2C, etc., etc.)

[dhalbert]

@GathererA Have you pushed these latest changes to your fork? Which board are you compiling for?

[v923z]

@dhalbert : is there a circuitpython programming manual somewhere, so that I could lend @GathererA a hand with this?

There is not really, any more than MicroPython has a manual. There is an older writeup here https://circuitpython.readthedocs.io/en/latest/docs/common_hal.html of how we structure the source tree.

OK, thanks! I will try to compile a user-defined module for circuitpython. I will start with the unix port, since this issue is not related to hardware at all.

[dhalbert]

OK, thanks! I will try to compile a user-defined module for circuitpython. I will start with the unix port, since this issue is not related to hardware at all.

We have barely changed the unix port at all, just enough so we can use its tests. and I'm not sure whether the C module user-defined module infrastructure is as you expect, because our last pull from upstream was around v1.9.4.

[v923z]

We have barely changed the unix port at all, just enough so we can use its tests. and I'm not sure whether the C module user-defined module infrastructure is as you expect, because our last pull from upstream was around v1.9.4.

Yeah, that's a bit unfortunate, but I developed user-defined modules for version 1.8, so I am somewhat hopeful.

[GathererA]

I've just pushed my latest edits to my fork of CircuitPython: https://github.com/spacecraft-design-lab-2019/circuitpython

@v923z I've only been trying to implement ndarray (and not even with all the functionality yet, as I've avoided rawsize and the binary operations as well). I can help you with the implementation into CircuitPython, since you'll need files in shared-bindings as well as shared-module (or common-hal), and you'll have to change a bunch of make files (somewhat outlined here: https://github.com/elvis-epx/circuitpython/blob/master/docs/common_hal.md). I haven't tried compiling for the unix port.

@dhalbert I'm building on a kicksat-sprite (since I'm working with Max Holiday at Stanford) that runs a SAMD51. Here's what the CircuitPython editor shows (with it ending in a crash). What's a J-Link?

[GathererA]

@v923z I'd love to assist how I can with the implementation, right now the only class that I've loaded as part of the ulab bundle is ndarray.c: https://github.com/spacecraft-design-lab-2019/circuitpython/blob/master/shared-bindings/ulab/ndarray.c

[GathererA]

Based on the fact that the ulab version isn't correct, I'm assuming that its saving the class to an unknown place in memory, so could it be an issue with QSTR?

[dhalbert]

@GathererA A J-Link is a hardware debugger: https://www.adafruit.com/?q=j-link. We use these all the time.

[dhalbert]

Based on the fact that the ulab version isn't correct, I'm assuming that its saving the class to an unknown place in memory, so could it be an issue with QSTR?

Yes, seems like some kind of build problem, and maybe the class initialization is wrong in some way. We have never used USER_C_MODULES in CircuitPython.

[v923z]

Based on the fact that the ulab version isn't correct, I'm assuming that its saving the class to an unknown place in memory, so could it be an issue with QSTR?

Yes, seems like some kind of build problem, and maybe the class initialization is wrong in some way. We have never used USER_C_MODULES in CircuitPython.

Would we learn anything from compiling circuitpython for the unix port? I have only a pyboard v.1.1 at hand, so I can test only on that, and linux.

I'd love to assist how I can with the implementation, right now the only class that I've loaded as part of the ulab bundle is ndarray.c: https://github.com/spacecraft-design-lab-2019/circuitpython/blob/master/shared-bindings/ulab/ndarray.c

Till we understand what exactly the problem is, we really have to strip this to the bare minimum. Obviously, if we can't print out the version number, we can't expect the library to work. The version number is saved in ROM as in https://micropython-usermod.readthedocs.io/en/latest/usermods_07.html#arbitrary-keyword-arguments, which means that the example there (https://github.com/v923z/micropython-usermod/blob/master/snippets/arbitrarykeyword/arbitrarykeyword.c) should also work. I see absolutely no point in trying to fix ulab before arbitrarykeyword.c, and simpleclass.c can be compiled simultaneously on both circuitpython, and micropython.

I don't insist on using a float as the version number, so that should not be a show stopper, but we have to be able to print out the array, at least. Does your firmware hang, if you do something like this:

import ulab as np
a = np.array([1, 2, 3])
print(a)
b = a + a
print(b)

or do you have difficulties only with dir(b)?

[dhalbert]

Would we learn anything from compiling circuitpython for the unix port? I have only a pyboard v.1.1 at hand, so I can test only on that, and linux.

I don't think it would be that helpful, because we only use the unix version for core language testing. Its build mechanism is pretty different than the usual ports.

The floating point version number means something is seriously messed up with pointers, so I certainly wouldn't just live with that. It's just the first obvious error that's showing up.

When adding a module to circuitpython, does that have to be both in shared-bindings, and in shared-modules at the same time? I see that you have it at both places.

shared-bindings contains the Python-visible API. shared-modules contains the port-independent part of the the actual implementation of the module. If there was a port-specific part of the module implementation, then it would be in ports/<some-port>/common-hal/.

So for ulab, unless there was something port-specific (like an FFT helper instruction for a peculiar chip), all the back-end code would be in shared-modules. For an I2C implementation, on the other hand, there'd be nothing in shared-modules, because all the ports need different chip-specific implementations.

shared-bindings + shared-modules takes the place of extmod/, essentially. In extmod, the Python API and the module implementation are mixed together, e.g., for something like ujson. We haven't given up on extmod completely, but we're trying to move things out of there. For instance, for us, ujson and ure (named as json and re in our module namespace) are still in extmod.

So a starting point would be just to leave this in extmod for now, or the equivalent. Eventually it could be refactored into the Python side API and the back-end implementation.

[v923z]

I have just looked at @GathererA 's code, and I see that porting a user-defined module to circuitpython is a major undertaking, even if the code does not concern the hardware. I am a bit skeptical now: I no longer think that we can get away with a couple of pre-processor macros.

[dhalbert]

We could be sloppy for now and just put everything in shared-bindings/, ignoring the stylistic guidance of separating the API from the implementation. If necessary, the library files could be #include'd wholesale from another directory. The point now is to try to test it and get it working at all.

[GathererA]

Thanks for all the info dhalbert! If it's okay with y'all, I think I'd like to continue to just put everything in shared-bindings until I get a working copy, then port the code over piece by piece to shared-module. Any clue as to where I should start looking into such an obvious pointer error?

@v923z to answer your question: no, the board breaks whether I use dir(np.array([1]) or just print(np.array([1]))

[dhalbert]

OK, I took a brief look at this. The version number printing problem is because the ulab code is assuming a certain MicroPython object representation. The available representations are MICROPY_OBJ_REPR_A, _B, _C, and _D. C and D encode floating point numbers directly and do not box them in a regular object: C uses 30-bit floats, and D uses 64-bit floats, and uses illegal float values to encode pointers and smallints. This is all explained in mpconfig.h. CircuitPython uses _C. This line in the ulab code:

mp_obj_float_t ulab_version = {{&mp_type_float}, ULAB_VERSION};

only works for A and B.

I am looking further at ulab.array(). It appears the constructor is returning a bad object. More later.

[dhalbert]

@GathererA You added bool deinited; to ulab_ndarray_obj_t, at the beginning of the struct. That broke the assumption by MicroPython/CircuitPython that the first struct field is the object type, and caused the object to be malformed.

Just remove that, and also remove all the deinit() and deinited() stuff. You don't need that: it exists only for on-chip peripherals and the like that represent scarce resources that must be released. It is not necessary for ndarray or anything else in ulab. I took that stuff out and was able to create and print an ndarray.

Adafruit CircuitPython 5.0.0-beta.2-30-g39df560fb-dirty on 2020-01-04; Adafruit Metro M4 Express with samd51j19
>>> import ulab as np
>>> a = np.array([33,44])
>>> a
array([33.0, 44.0], dtype=float)

I believe you could get away without doing the extensive file refactoring of the ulab source you have been doing as well. The important things are redoing the function signatures of a few functions, but the file structure is not required to be reworked for inclusion. Consider adding ulab as another set of files in extmod/, say.

[GathererA]

Wow, taking the deinit bool out fixes the print issue, thanks!!! As for declaring the version as a float, what is the most appropriate way?

float ulab_version = 0.262; STATIC const mp_rom_map_elem_t ulab_globals_table[] = { { MP_ROM_QSTR(MP_QSTR___name__), MP_OBJ_NEW_QSTR(MP_QSTR_ulab) }, { MP_ROM_QSTR(MP_QSTR___version__), MP_ROM_PTR(&ulab_version) }, { MP_OBJ_NEW_QSTR(MP_QSTR_array), (mp_obj_t)&ulab_ndarray_type }, { MP_ROM_QSTR(MP_QSTR_uint8), MP_ROM_INT(NDARRAY_UINT8) }, { MP_ROM_QSTR(MP_QSTR_int8), MP_ROM_INT(NDARRAY_INT8) }, { MP_ROM_QSTR(MP_QSTR_uint16), MP_ROM_INT(NDARRAY_UINT16) }, { MP_ROM_QSTR(MP_QSTR_int16), MP_ROM_INT(NDARRAY_INT16) }, { MP_ROM_QSTR(MP_QSTR_float), MP_ROM_INT(NDARRAY_FLOAT) }, };

Doesn't work but not sure how else to initialize it

[dhalbert]

I would suggest making the version a string, not a float, to have control over the formatting. E.g., is it "1.10" or "1.1"? And if one wanted to use semver-style, x.x.x, then it can't be a float.

[GathererA]

Thanks for all the help guys! I've got ulab working in a basic format with ndarray and parts of linalg, and I've pushed it on my forked circuitpython repo. Will continue to add features and document better. Ultimately would like to change the code to also conform to circuitpython coding practice with a shared-module as well as a shared-bindings folder

[v923z]

I would suggest making the version a string, not a float, to have control over the formatting. E.g., is it "1.10" or "1.1"? And if one wanted to use semver-style, x.x.x, then it can't be a float.

This is a very good point, thanks for bringing it up. I would then suggest something along the lines

STATIC MP_DEFINE_STR_OBJ(ulab_version_obj, "0.26.4");

STATIC const mp_map_elem_t ulab_globals_table[] = {
...
    { MP_ROM_QSTR(MP_QSTR___version__), MP_ROM_PTR(&ulab_version_obj) },
...

@GathererA, could you, please, check, whether this is compatible with circuitpython? I could compile it for micropython, and got the expected results.

Thanks for all the help guys! I've got ulab working in a basic format with ndarray and parts of linalg, and I've pushed it on my forked circuitpython repo.

This is great, though, I have mostly been just cheering you on from the sideline...

Ultimately would like to change the code to also conform to circuitpython coding practice with a shared-module as well as a shared-bindings folder

Is there some common ground that we could establish? I do think that it is unreasonable to maintain two versions of the same code, and we should attempt to try get something that is acceptable for everyone.

For one thing, I feel that circuitpython's code is a bit too verbose in the comments, to an extent that makes it a bit unpleasant to read. I know that the reason for that is that circuitpython generates the documentation from the source code itself, but that actually does not happen. So, if we end up with a single code base for both circuitpython, and micropython, then I would like to code to look like micropython code. If you need documentation for it, you can use a jupyter notebook, so that you still have tight coupling between code, and documentation.

@GathererA, should we consider the issue resolved? We can keep it open, if you want.

[dhalbert]

FYI, we (the CircuitPython team) forked ulab, made a few minor changes, and have a PR to CircuitPython to include it. We're reformatting the doc to link into ours and use standard function signatures, etc. @jepler is doing the work: thanks @jepler! He got it going quickly. If you want to try it, there are build artifacts under the dropdown on the right-hand side of https://github.com/adafruit/circuitpython/pull/2583/checks.

We can figure out how to PR some of this back to you.

We tried to add it to kicksat_sprite but there isn't enough room. It takes about 35kB, it looks like.

EDIT: FFT in use: https://blog.adafruit.com/2020/02/04/its-fft-tuesday-testing-out-mic-fft-on-circuitpython-circuitpython-adafruit-fft/

[v923z]

Dan,

Many thanks for the update!

FYI, we (the CircuitPython team) forked ulab, made a few minor changes, and have a PR to CircuitPython to include it. We're reformatting the doc to link into ours and use standard function signatures, etc. @jepler is doing the work: thanks @jepler! He got it going quickly. If you want to try it, there are build artifacts under the dropdown on the right-hand side of https://github.com/adafruit/circuitpython/pull/2583/checks.

We can figure out how to PR some of this back to you.

OK, let me know if you can come up with a reasonable scheme, I would be more than happy to incorporate your modifications. It doesn't even need to be a proper PR. Do you think that we can somehow establish a code base that is compatible with both micropython, and circuitpython?

We tried to add it to kicksat_sprite but there isn't enough room. It takes about 35kB, it looks like.

I think I know where the culprit is: the binary_op, where most of the things happen in macros. An alternative would be to do everything in float, and then convert at the end to the required (upcast) result. That would incur a performance hit, because the conversion to float requires a function call for each item.

As I pointed out elsewhere, it would be easy to make the code modular (I am working on it), so one could strip the unnecessary parts. The difficulty with that approach is that the special functions, like FFT, matrix inversion, polyfit etc. don't actually take so much space. One loses the RAM in the base class, ndarray. We could consider making ndarray itself configurable, but I don't know what a reasonable cut would be. Do you have a feel as to which unary/binary operators people would use?

EDIT: FFT in use: https://blog.adafruit.com/2020/02/04/its-fft-tuesday-testing-out-mic-fft-on-circuitpython-circuitpython-adafruit-fft/

Oh gosh, this is sweet! I had exactly this in mind, when I started to write the FFT code. Fantastic!

[GathererA]

Wow, @dhalbert this is amazing! Thank you so much for the update! I'm sorry for the delay, I've been swamped since the beginning of school again. We've been updating our own bespoke version of ulab with some other functionality like norms, cross products, simple stuff. Since we only need linalg and ndarray in the ulab repo, we've just loaded those onto the board and it barely fits! What would be the best way for us to go about integrating the other functions we've created (if you think that would even be a good idea) down the line?

[jepler]

Ideally, when it comes to adding functionality, you could PR it to micropython-ulab and then we could merge that into circuitpython-ulab. It means more work for circuitpython centric people (for instance to first add without translate() and then add it in when merging), but it's fairest to upstream.

[v923z]

Since we only need linalg and ndarray in the ulab repo, we've just loaded those onto the board and it barely fits! What would be the best way for us to go about integrating the other functions we've created (if you think that would even be a good idea) down the line?

@GathererA First, if you have new functionality, let us have it! You implemented those, because you found use for them, but then chances are, it could help others, too. So, please, let us include those functions! Second, @dhalbert also indicated that space is a concern. As I said earlier, I am working on a more modular design, so one could then choose which functions should be compiled into the firmware. But on top of that, I wanted to ask, whether you need matrices. As a testbed, I have developed a 1D implementation. That is significantly simpler, consumes less flash, because we can yank the 2D stuff, and it is also easier on RAM, because it works with views, and not complete copies of ndarrays. I assume, most people need 1D arrays only, anyway. Give me a day or two, and I can push it to the repository.

Ideally, when it comes to adding functionality, you could PR it to micropython-ulab and then we could merge that into circuitpython-ulab.

I would prefer that too, otherwise, resources will be fragmented.

@jepler

It means more work for circuitpython centric people (for instance to first add without translate() and then add it in when merging), but it's fairest to upstream.

I am willing to insert a do-nothing translate macro for the micropython case, so you wouldn't have to deal with that.

[v923z]

@dhalbert Dan, I am trying to add the empty translate macro to micropython. You have mentioned somewhere that circuitpython makes a CIRCUITPY or similar flat available, but I don't find your comment on this. Can you help out with that?

[dhalbert]

@v923z In py/circuitpy_mpconfig.h we #define CIRCUITPY 1. That include file is included in mpconfigport.h for all the ports we support. So you can just #ifdef CIRCUITPY to check if it's a CircuitPython build.