v923z / micropython-ulab

a numpy-like fast vector module for micropython, circuitpython, and their derivatives
https://micropython-ulab.readthedocs.io/en/latest
MIT License
430 stars 117 forks source link

[BUG] Compile error building ports/unix from CircuitPython (8de9d5a52) - uses ulab (6619c20) #635

Closed kbsriram closed 1 year ago

kbsriram commented 1 year ago

Describe the bug building ports/unix from CircuitPython results in a build error (tested close to current head at 8de9d5a52 - it uses ulab 6619c20)

To Reproduce

git clone https://github.com/adafruit/circuitpython cp_test
cd cp_test
make -C mpy-cross
make fetch-all-submodules
cd ports/unix
make axtls
make micropython

Results in the following error when running on an ubuntu machine 4.15.0-211-generic #222-Ubuntu SMP Tue Apr 18 18:55:06 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

In file included from ../../extmod/ulab/code/numpy/vector.c:15:0:
../../extmod/ulab/code/numpy/vector.c: In function ‘ulab_sinc1’:
../../extmod/ulab/code/numpy/vector.c:580:20: error: conversion to ‘float’ from ‘mp_float_t {aka double}’ may alter its value [-Werror=float-conversion]
     if (fpclassify(x) == FP_ZERO) {
                    ^
../../extmod/ulab/code/numpy/vector.c:580:20: error: conversion to ‘float’ from ‘mp_float_t {aka double}’ may alter its value [-Werror=float-conversion]
     if (fpclassify(x) == FP_ZERO) {
                    ^
cc1: all warnings being treated as errors
../../py/mkrules.mk:62: recipe for target 'build-standard/extmod/ulab/code/numpy/vector.o' failed

Additional context It looks like #617 introduced fpclassify rather than MICROPY_FLOAT_CONST for a comparison, which gives trouble for builds where MICROPY_FLOAT is a native double, and hasn't been explicitly overridden with MICROPY_FLOAT_IMPL_FLOAT. If the simplest fix is just to use MICROPY_FLOAT_CONST(0.0) here, lmk if you prefer a fix PR :-)

v923z commented 1 year ago

Thanks for bringing it up! I think the solution, as you suggest, is something along these lines:


static mp_float_t vector_sinc1(mp_float_t x) {
    if (x == MICROPY_FLOAT_CONST(0.0)) {
        return MICROPY_FLOAT_CONST(1.);
    }
    x *= MP_PI;
    return MICROPY_FLOAT_C_FUN(sin)(x) / x;
}

If that's indeed the case, please, open a PR!