-
Notifications
You must be signed in to change notification settings - Fork 57
Extract quantized biases for Conv/ConvTranspose #224
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
Conversation
|
I am not sure the failing test has something to do with my code changes, would anyone mind re-running it to check? |
|
I restarted the tests and they seem to work. |
|
The example we have from https://github.com/fastmachinelearning/qonnx/blob/custom/SIRA/src/qonnx/transformation/extract_conv_bias.py also handles the cases (or really exits gracefully) when the quant input and scale are not initializers. Would it be useful to also add it here? Also, the zero offset seems to be ignored. It seems like it shouldn't be. In general, though, does the extracted bias remain quantized, or do we drop quantization? |
Great to hear there’s already a related solution on another branch. My main goal here is to close the design gap where quantized biases aren’t being extracted. I’m happy for this to be solved in any way that fits the project best. Feel free to modify my PR directly or tell me what exactly you would like to see differently, use the implementation from the other branch or implement an alternative approach if that’s cleaner. To answer your question: yes, in my implementation the extracted bias remains quantized. The idea is to extract the bias into an Add node placed after the Conv or ConvTranspose, and then connect the parameter input of the Add node directly to the Quant node that produces the bias. In some cases a reshape is required. |
|
To get this merged quickly, I think it is fine to stop with the quantized constant bias for now. We can still extend this later or merge in the solution from the SIRA branch, which unfortunately still requires some more effort to get this merge ready... Also adding support for float quantizers could be interesting in the future, but for now, if you could have a look at the zero-point input, we can merge. |
This PR updates the
ExtractBiasFromConvtransform so it also handles quantized biases. Previously, extraction only worked when the bias was a static initializer on input[2], which is not the case once the bias itself is quantized (it becomes a dynamic input produced by a Quant node). This issue is documented here: Xilinx/finn#1484The PR also contains a new test case to test bias extraction for Conv and ConvTranspose layers with no bias quantization, per‑tensor bias quantization and per‑channel bias quantization.
Per‑channel bias quantization essentially is like having a float bias, since each bias element gets its own scale. Still, some quantization setups produce this pattern, so the transform needs to support it.