Set eps in end-to-end QAT flow #2180
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/2180
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit 4cc36b8 with merge base c9b9adc ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
| @@ -539,7 +540,8 @@ def get_group_qparams_symmetric( | |||
| assert n_bit <= 8, f"unsupported n_bit: {n_bit}" | |||
|
|
|||
| block_size = (1, groupsize) | |||
| eps = torch.finfo(w.dtype).eps | |||
| if eps is None: | |||
| eps = torch.finfo(w.dtype).eps | |||
There was a problem hiding this comment.
Isn't this already the behavarior inside choose_qparams_affine?
There was a problem hiding this comment.
Yeah, I could just change FakeQuantizer to use that instead, but seems like a bigger change, prefer to leave that in a future PR to keep this one isolated
andrewor14
force-pushed
the
qat-eps
branch
2 times, most recently
from
13d9cb2 to
f782422
Compare
andrewor14
added
the
topic: improvement
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
andrewor14
force-pushed
the
qat-eps
branch
2 times, most recently
from
eb6431c to
17e9ccb
Compare
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@andrewor14 maybe we can relax the failing test_export_compile_aoti_PackedLinearInt8DynamicActivationIntxWeightLayout test to just look for the op name? |
Looks like the actual failing test is |
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
**Summary:** This commit does two things:
(1) Allow users to set eps in `FakeQuantizeConfig`
(2) For other parts of the QAT flow, set eps to
`torch.finfo(torch.float32).eps` for input linear
activations to match the existing hardcoded input
activation scale dtype (which is fp32)
The motivation is to enable users who wish to lower their
models to XNNPACK. This would require them to use the following
combination of dtypes during training for end-to-end numerical
match:
- input activations: bf16
- input activation scales: fp32
- input activation eps: `torch.finfo(torch.float32).eps`
- weight: bf16
- weight scales: bf16
- weight eps: `torch.finfo(torch.bfloat16).eps`
However, today there is no way to specify the above in any
of the QAT flows. For the recommended `FakeQuantizeConfig`
flow, we always use `torch.finfo(x.dtype).eps`, where x is
bf16 in this case, and there is no way for users to configure
this. This is resolved by (1).
For the legacy `Int8DynActInt4QATQuantizer` flow, we hardcode
input activation scales to always use fp32 in
#2085, but did not set the
corresponding eps. Today, this also uses `torch.finfo(x.dtype).eps`
by default, where x is bf16, and so we use the wrong eps value.
This is resolved by (2).
**Test Plan:**
python test/quantization/test_qat.py -k test_fake_quantize_config_eps
python test/quantization/test_qat.py -k test_qat_8da4w_eps
|
Hey @metascroy, looks like there's a new test failure and it seems real? I think it's complaining that qdq ops are not fused. I wonder if it's because there's a new eps value that broke the pattern matching somehow. Can you take a look? |
andrewor14
changed the title
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Traveling today, but I'll have a look later in afternoon. |
Pushed a fix to your PR |
|
@andrewor14 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Ok, remaining test failure seems unrelated and happens on other PR. I think this is good to land. |
Summary: This commit does two things:
(1) Allow users to set eps in
FakeQuantizeConfig(2) For other parts of the QAT flow, set eps to
torch.finfo(torch.float32).epsfor input linear activations to match the existing hardcoded input activation scale dtype (which is fp32)The motivation is to enable users who wish to lower their models to XNNPACK. This would require them to use the following combination of dtypes during training for end-to-end numerical match:
torch.finfo(torch.float32).epstorch.finfo(torch.bfloat16).epsHowever, today there is no way to specify the above in any of the QAT flows. For the recommended
FakeQuantizeConfigflow, we always usetorch.finfo(x.dtype).eps, where x is bf16 in this case, and there is no way for users to configure this. This is resolved by (1).For the legacy
Int8DynActInt4QATQuantizerflow, we hardcode input activation scales to always use fp32 in#2085, but did not set the corresponding eps. Today, this also uses
torch.finfo(x.dtype).epsby default, where x is bf16, and so we use the wrong eps value. This is resolved by (2).Test Plan:
python test/quantization/test_qat.py -k test_fake_quantize_config_eps
python test/quantization/test_qat.py -k test_qat_8da4w_eps