numpy/vector.c: remove usage of fpclassify

[kbsriram]

Fixes #635

Verified by re-compiling circuitpython with this change.

[jepler]

Under the C standard, I think both the original code and the replacement code should give correct results.

According to the C99 standard, the fpclassify macro accepts arguments of any "real floating-point type". That is, arguments of "float" or "double" type are both equally appropriate.

Of course, we have to live with compilers actually "in production". That said, based on your kernel version I checked whether Ubuntu 18.04 gave such an error and I didn't reproduce it. I am curious what specific compiler you are using.

When I implemented ulab.numpy.sinc I chose to not perform a comparison against the floating point constant zero because in CircuitPython we prefer to enable the "-Werror=float-equal" setting to treat any direct floating-point equality comparisons as errors. However, we do already disable this diagnostic in all ulab files, because ulab does not follow the same coding standard, so making this change does not affect whether CircuitPython builds properly.

The failed builds appear to be due to a bug in the CI commands in ulab. Per documentation of github actions, it is needed to run "apt-get update" before "apt-get install". actions/runner-images#2924

[kbsriram]

Right - maybe fpclassify is triggering an unrelated issue that was never exercised earlier. It looks like a similar issue was also encountered in micropython - micropython/micropython@f31f9a8

Re: compiler version and possible options triggering the issue - it looks like the CP build disables float-equal but the float-conversion option remains, which is causing this compiler version to be unhappy.

$ gcc --version
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0

The full command that fails for this version:

gcc -I../../lib/berkeley-db-1.xx/PORT/include -I. -I../.. -Ibuild-standard -I../../shared/readline -Wall -Werror -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Wpointer-arith -Wdouble-promotion -Wfloat-conversion -std=gnu11 -DUNIX -DCIRCUITPY_ULAB=1 -DMODULE_ULAB_ENABLED=1 -DULAB_HAS_USER_MODULE=0 -iquote ../../extmod/ulab/code -DFFCONF_H=\"lib/oofatfs/ffconf.h\" -DMICROPY_VFS_LFS1=0 -DMICROPY_VFS_LFS2=0 -DMICROPY_PY_BTREE=1 -DMICROPY_USE_READLINE=1 -DMICROPY_PY_TERMIOS=1 -DMICROPY_PY_THREAD=1 -DMICROPY_PY_THREAD_GIL=0  -DMICROPY_PY_FFI=1 -Os -DNDEBUG -fdata-sections -ffunction-sections -Ivariants/standard  -g -U _FORTIFY_SOURCE -DMICROPY_QSTR_EXTRA_POOL=mp_qstr_frozen_const_pool -DMICROPY_MODULE_FROZEN_MPY -DMICROPY_MODULE_FROZEN_STR -DMPZ_DIG_SIZE=16  -DMICROPY_MODULE_FROZEN_STR -Wno-missing-declarations -Wno-missing-prototypes -Wno-unused-parameter -Wno-float-equal -Wno-sign-compare -Wno-cast-align -Wno-shadow -DCIRCUITPY -c -MD -o build-standard/extmod/ulab/code/numpy/vector.o ../../extmod/ulab/code/numpy/vector.c
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) {

I was also able to narrow the issue to being triggered when using -Os when using this version of gcc.

$ cat test.c 
#include <math.h>

int f(double a) {
  return fpclassify(a);
}
$ gcc -Werror -Wfloat-conversion -Os -c test.c 
In file included from test.c:1:0:
test.c: In function ‘f’:
test.c:4:21: error: conversion to ‘float’ from ‘double’ may alter its value [-Werror=float-conversion]
   return fpclassify(a);
                     ^
test.c:4:21: error: conversion to ‘float’ from ‘double’ may alter its value [-Werror=float-conversion]
   return fpclassify(a);
                     ^
cc1: all warnings being treated as errors
$ gcc -Werror -Wfloat-conversion -c test.c 
$ 

Testing other options that seemed suspicious (like -std=gnu11 vs -std=gnu99 and -std=c99) still trigger the error whenever -Os is specified. Fwiw, compiling with g++ was always successful (after removing the C related options) but this might not be very useful, and unsurprisingly, disabling float-conversion warnings also worked fine.

[v923z]

This fails on the micropython build, and cannot be merged before that's sorted out.

[kbsriram]

@v923z - I merged the CI fix in d025aa3 (/ht @jepler) and hopefully succeeds on the next run!

[jepler]

Ah, OK, adding -Os I do reproduce the diagnostic on gcc even up to trunk(!!) https://godbolt.org/z/Mh9c5rnE6

It's unfortunate, because fpclassify is a useful function and when implemented according to the C99 standard does not perform any float conversion.

I don't expect prompt action but I did file a bug with the gcc folks related to this: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110703 -- there are several similar bugs that have not seen action over the course of years, unfortunately.

[v923z]

@v923z - I merged the CI fix in d025aa3 (/ht @jepler) and hopefully succeeds on the next run!

Yes, it did pass, many thanks for your contribution!