Skip to content
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

Handle ExtractConvBias properly and remove unused bias Quant nodes #122

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

joannapng
Copy link

During the ExtractBiasFromConv transformation, sometimes the bias initializer was not found. The current code searches for the bias initializer with the name of the tensor that acts as input to the convolution node, but it should search for the initializer with the name of the first input to the Quant node that produces this tensor. For the same reason, the producer node should be deleted and not the tensor.

@maltanar
Copy link
Collaborator

maltanar commented Aug 13, 2024

Hi @joannapng , thanks for the PR! It would be nice to be able to handle quantized biases with ExtractConvBias, but there's a few problems that need to be fixed first.

  1. Your changes break the transformation for Conv nodes with non-quantized biases, the second line here accesses producer.input without checking if producer is None which can happen for non-quantized biases:
                        producer = model.find_producer(n.input[2])
                        bias = model.get_initializer(producer.input[0])
  1. To handle quantized biases correctly, the second input to the Add node we insert should be the quantized bias (the output of the Quant node for the bias) and not the un-quantized input to the bias quantizer itself. It seems you are currently removing the Quant node entirely, which would not preserve the semantics of the original network: so currently rewriting Conv(x, bias=Quant(unquant_b)) to Conv(x) + unquant_b but it should be Conv(x) + Quant(unquant_b) instead.

If you could make this fix and perhaps add a unit test that checks both the quantized and unquantized bias cases, that would be great!

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.

2 participants