-
Notifications
You must be signed in to change notification settings - Fork 806
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
Adds FFT Auto Scale Op #2134
Adds FFT Auto Scale Op #2134
Conversation
in_frame = np.array([int(j) for j in lines[i].split()], dtype=np.int16) | ||
out_frame_exp = [int(j) for j in lines[i + 1].split()] | ||
scale_exp = [int(j) for j in lines[i + 2].split()] | ||
# TFLite |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFLM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Changed to TFLM, but we have a TODO elsewhere to revisit these tests in general when we get the chance.
#include "signal/src/msb.h" | ||
|
||
// TODO(b/286250473): remove namespace once de-duped libraries | ||
namespace tflm_signal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this in tflite::tflm_signal
? When we remove the namespace, we don't want the symbol to wind up in the global namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. I wonder if we can remove the tflm_signal anyways then, or just reword it to tflite when we de-dupe?
signal/src/fft_auto_scale.cc
Outdated
namespace tflm_signal { | ||
|
||
int FftAutoScale(const int16_t *input, int size, int16_t *output) { | ||
const int16_t max = tflm_signal::MaxAbs16(input, size); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tflm_signal::
prefix shouldn't be necessary here or line 29, as we're already in that namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
#include "signal/src/max_abs.h" | ||
|
||
// TODO(b/286250473): remove namespace once de-duped libraries | ||
namespace tflm_signal { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here about putting it in the tflite::tflm_signal
namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
This PR adds additional FFT op functionality in the Signal library, namely adding the FFT Auto Scale operation.
Testing added in the original
fft_test.cc
andfft_ops_test.py
.BUG=287346710