Skip to content

Conversation

@auphelia
Copy link
Collaborator

@auphelia auphelia commented Jan 6, 2026

This PR updates the ExtractBiasFromConv transform 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#1484

The 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.

@auphelia
Copy link
Collaborator Author

auphelia commented Jan 6, 2026

I am not sure the failing test has something to do with my code changes, would anyone mind re-running it to check?

@jmitrevs
Copy link
Collaborator

jmitrevs commented Jan 6, 2026

I restarted the tests and they seem to work.

@jmitrevs
Copy link
Collaborator

jmitrevs commented Jan 6, 2026

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?

@auphelia
Copy link
Collaborator Author

auphelia commented Jan 7, 2026

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.

@auphelia auphelia requested a review from iksnagreb January 23, 2026 14:34
@iksnagreb
Copy link
Collaborator

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.

@iksnagreb iksnagreb added this to the v1.0.0 milestone Jan 23, 2026
@auphelia auphelia requested a review from iksnagreb January 23, 2026 16:46
@jmitrevs jmitrevs merged commit 3d87295 into main Jan 29, 2026
3 checks passed
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.

3 participants