-
Notifications
You must be signed in to change notification settings - Fork 452
activation: use dedicated fixed-point λ for SeLU (fixes #1287) #1296
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
base: main
Are you sure you want to change the base?
Conversation
…arning#1287) * Replaced literal cast 1.050700987… → `res_T` with a `static const ap_fixed<16,6> lambda`, preserving range and ~1.5 × 10⁻² LSB precision regardless of user-chosen `res_T`. * Removed redundant datareg scope; now a single per-element branch: if (x ≥ 0) y = λ · x else y = selu_table[idx] (with index clamped to [0,N-1]). * Guard against negative-index underflow; behaviour for <0 inputs unchanged. * Keeps identical latency/area on Vivado 2023.1 & Vitis 2024.1, but eliminates silent overflow/rounding when `res_T` is narrow (e.g., ap_fixed<8,2>). Fixes fastmachinelearning#1287
Why use 6 integer bits to represent "1.0507"? |
Yeah, my bad. It was late and I must have confused with another project of mine. I'm updating it to use 2 integer bits. |
One could also use ufixed since the value is positive, to not use a sign bit. |
Can you also fix the pre-commit issue? (I think it's trailing whitespace) |
@@ -717,9 +717,9 @@ template <class data_T, class res_T, typename CONFIG_T> void selu(data_T data[CO | |||
initialized = true; | |||
} | |||
|
|||
typedef ap_fixed<16,2> selu_const_t; // ±512 range, ≈1.5e-2 LSB | |||
typedef ap_ufixed<16,2> selu_const_t; |
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.
Would just 1 integer bit be enough?
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.
yeah, kept one more just to be sure, do you want me to bring it to 1?
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.
I think 1 makes sense, since the number is in the range.
For pre-commit, add the pre-commit package to your python setup and do |
guys it still finds trailing whitespaces, but I can't find them, any suggestions? |
guys I'm opening a new PR since this one is getting messy |
res_T
with astatic const ap_fixed<16,2> lambda
, preserving range and ~1.5 × 10⁻² LSB precision regardless of user-chosenres_T
.else y = selu_table[idx] (with index clamped to [0,N-1]).
eliminates silent overflow/rounding when
res_T
is narrow (e.g., ap_fixed<8,2>).Fixes #1287