Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

fix wrong quantization target in weight quantization #4038

Merged
merged 9 commits into from
Aug 24, 2021

Conversation

chenbohua3
Copy link
Contributor

This pr contains two things:

  1. When bn fold is enable, the folded weight is assign to module.weight, QAT quantizer should quantize module.weight instead of module.old_weight
  2. pass the weight that should be quantized to each quantizer directly to make the training graph more clearly.

@@ -221,15 +220,13 @@ def quantize_input(self, *inputs, wrapper, **kwargs):
self.record(wrapper, 'input', inputs)
return inputs

def quantize_weight(self, wrapper, **kwargs):
def quantize_weight(self, weight, wrapper, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain more about this change? weight can be obtained from wrapper, why pass it again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's about the code readability. Since we already pass the weight to be quantized (new_weight) to quant_grad here, it is better to directly use it instead of obtaining it from wrapper. The developer can easily know that quantize_weight is a big op to simulate quantization and the op takes origin/bn-folded weight as input. I think using wrapper.weight will make it difficult to understand to structure of the training graph.

@linbinskn
Copy link
Contributor

LGTM. Since this PR also changes quantize_weight in DoReFa and BNN quantizer, please also test them together.

@chenbohua3
Copy link
Contributor Author

Have added a ut about the interface of quantize_weight

@chenbohua3 chenbohua3 changed the title pass weight to quantize_weight to make the graph more clearly fix wrong quantization target in weight quantization Aug 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants