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

Allow setting a suffix for constants and trigonometric functions #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

steffen-kiess
Copy link
Contributor

This is similar to #12, but for the C version. The C version uses sin() and cos() and constants (for pi) which are not accurate enough for long double.

In order to use constants or trigonometric functions with a type other than
double, a suffix ('f' for float or 'l' for long double) has to be used in C.
This commit adds a preprocessor macro 'kiss_fft_suffix' which can be set to
either 'f' or 'l' and which will be added to floating point constants and to
the trigonometric functions (sin and cos).

Without this suffix, the code will use a too high precision for float and a
too low precision for long double.

@mborgerding
Copy link
Owner

Thanks for the input. I think this will improve accuracy for long double.
However, I think this can (and should) be accomplished without requiring any new build flags.
Maybe define KISS_FFT_COS and KISS_FFT_SIN differently depending on the kiss_fft_scalar definition?

@steffen-kiess
Copy link
Contributor Author

Comparing strings in the C preprocessor is rather difficult, especially because long double consists of two words. The following code would work, but is a rather ugly hack:

#define kiss_fft_scalar float
//#define kiss_fft_scalar double
//#define kiss_fft_scalar long double



#define X_float_X 1
#define X__X 2
#define X_long_X 3
#define concat2(a, b, c) a##b##c
#define concat(a, b, c) concat2(a, b, c)

#define double

#if concat(X_, kiss_fft_scalar, _X) == X_float_X
#define kiss_fft_suffix f
#elif concat(X_, kiss_fft_scalar, _X) == X__X
#undef kiss_fft_suffix
#elif concat(X_, kiss_fft_scalar, _X) == X_long_X
#define kiss_fft_suffix l
#else
#undef kiss_fft_suffix
#warning "Unknown kiss_fft_scalar type"
#endif

#undef double



#include <stdio.h>
#define tostr1(x) #x
#define tostr(x) tostr1(x)
int main () {
  printf("%s\n", tostr(kiss_fft_suffix));
}

@mborgerding
Copy link
Owner

@steffen-kiess Could you put the above preprocessor hack into a PR?

@steffen-kiess
Copy link
Contributor Author

I've updated this pull request.

In order to use constants or trigonometric functions with a type other than
double, a suffix ('f' for float or 'l' for long double) has to be used in C.
This commit adds a preprocessor macro 'kiss_fft_suffix' which can be set to
either 'f' or 'l' and which will be added to floating point constants and to
the trigonometric functions (sin and cos).

Without this suffix, the code will use a too high precision for float and a
too low precision for long double.
@steffen-kiess
Copy link
Contributor Author

The change was breaking USE_SIMD / FIXED_POINT, this should be fixed now. Currently both USE_SIMD and FIXED_POINT use double for floating point operations (like before), but this can be changed in kiss_fft.h.

@mborgerding
Copy link
Owner

Thanks for the update. Your preprocessor kung fu is strong. At times like this, I would reach for templates in a c++ project. I'll take a closer look and expect to merge this soon.

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

Successfully merging this pull request may close these issues.

2 participants