Fixes: quantized_relu & unsigned profiling
Created by: thesps
Note: bugfixes targeting release branch
quantized_relu
Since this change, we assign the wrong data type to quantized_relu activations. It sets the correct type for quantized_bits, just not quantized_relu. This is what causes the tests flagged in #377 to fail.
Using the example of the hls4ml-tutorial Part3 QKeras model which uses quantized_relu(6) activations, on master branch we assign ap_fixed<6,1>. But appropriate choices would be instead ap_fixed<7,1> (signed) or ap_ufixed<6,0> (unsigned). I've chosen to go for the ap_ufixed case for the fix since it uses fewer bits.
You can see that the types we currently assign to the relu layers are wrong in the profiling:
The model accuracy is:
- 0.743656 QKeras on CPU (reference / target)
- 0.743542 hls4ml on master branch
- 0.743855 hls4ml on this PR (identical if
ap_fixed<7,1>was used instead)
The synthesis results slightly prefer the ap_ufixed variation which has one fewer bit than the necessary ap_fixed option.
quantized_relu precision |
Accuracy | Latency | II | LUT | FF | DSP | BRAM18 |
|---|---|---|---|---|---|---|---|
ap_ufixed<6,0> |
0.743855 | 9 | 1 | 33706 | 3037 | 119 | 4 |
ap_fixed<7,1> |
0.743855 | 9 | 1 | 37523 | 3155 | 119 | 4 |
The quantized_relu tests now pass, so they are included again, resolving #377. I added a few more variations as well.
unsigned profiling
In fixing the above, I noticed that we don't display unsigned types correctly in the profiling, so I include the fix for that as well. (Separate commits in case they need to be taken separately).
Before (master):
After (this PR):
