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

Epsilon double promotion warning/error. #18

Closed
iabdalkader opened this issue Nov 14, 2019 · 4 comments
Closed

Epsilon double promotion warning/error. #18

iabdalkader opened this issue Nov 14, 2019 · 4 comments

Comments

@iabdalkader
Copy link

MICROPY_FLOAT_IMPL_FLOAT turns on -Wdouble-promotion and -fsingle-precision-constant which causes epsilon literal to be float and an implicit conversion from 'float' to 'double' (to match fabs) warning/error. Maybe fabsf should be used instead...

QSTR not updated
CC ../../extmod/ulab/code/linalg.c
../../extmod/ulab/code/linalg.c: In function 'linalg_invert_matrix':
../../extmod/ulab/code/linalg.c:127:32: error: implicit conversion from 'float' to 'double' to match other operand of binary expression [-Werror=double-promotion]
         if(fabs(data[m*(N+1)]) < epsilon) {
                                ^
../../extmod/ulab/code/linalg.c: In function 'linalg_det':
../../extmod/ulab/code/linalg.c:310:35: error: implicit conversion from 'float' to 'double' to match other operand of binary expression [-Werror=double-promotion]
         if(fabs(tmp[m*(in->n+1)]) < epsilon) {
                                   ^
../../extmod/ulab/code/linalg.c: In function 'linalg_eig':
../../extmod/ulab/code/linalg.c:349:24: error: implicit conversion from 'float' to 'double' to match other operand of binary expression [-Werror=double-promotion]
             if(epsilon < fabs(array[m*in->n + n] - array[n*in->n + m])) {
                        ^
cc1: all warnings being treated as errors
@v923z
Copy link
Owner

v923z commented Nov 14, 2019

I think this has been fixed in the testing branch. (All occurrences of fabs have been replaced by MICROPY_FLOAT_C_FUN(fabs). This is the proper way of handling this issue, because the definition of float is platform dependent: https://micropython-ulab.readthedocs.io/en/latest/ulab.html#ndarray-the-basic-container) Can you, please, compile against testing?

@iabdalkader
Copy link
Author

Yes this particular issue is fixed in testing, but in the testing branch numerical.c has other issues:

CC ../../extmod/ulab/code/vectorise.c
../../extmod/ulab/code/numerical.c: In function 'numerical_sum_mean_ndarray':
../../extmod/ulab/code/numerical.c:135:5: error: implicit declaration of function 'printf' [-Werror=implicit-function-declaration]
     printf("%ld, %ld\n", m, n);
     ^~~~~~
../../extmod/ulab/code/numerical.c:135:5: error: incompatible implicit declaration of built-in function 'printf' [-Werror]
../../extmod/ulab/code/numerical.c:135:5: note: include '<stdio.h>' or provide a declaration of 'printf'
../../extmod/ulab/code/numerical.c:135:15: error: format '%ld' expects argument of type 'long int', but argument 2 has type 'size_t {aka unsigned int}' [-Werror=format=]
     printf("%ld, %ld\n", m, n);
             ~~^
             %d
../../extmod/ulab/code/numerical.c:135:20: error: format '%ld' expects argument of type 'long int', but argument 3 has type 'size_t {aka unsigned int}' [-Werror=format=]
     printf("%ld, %ld\n", m, n);
                  ~~^
                  %d

Including stdio.h and using %zu fixes it.

Can the array fixes be merged in master ? I don't want to have to fork and maintain this repo.

@v923z
Copy link
Owner

v923z commented Nov 14, 2019

This printout was there for debugging purposes, and is not needed at all. I have removed it, and pushed the fix to master. Let me know if it still doesn't work. (I could compile it for the unix port.)

@iabdalkader
Copy link
Author

It builds now, thanks for fixing it!

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

No branches or pull requests

2 participants