-
Notifications
You must be signed in to change notification settings - Fork 280
Add support for bmm for fbgemm config #2337
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
base: main
Are you sure you want to change the base?
Conversation
Summary: att, this PR adds support for running quantized bmm, the quantized bmm kernel for int4 and fp8 (with dynamic activation quantization) requires transpose of weights in order to run, so added transpose_input to the convert function to transpose the weights first Test Plan: python test/dtypes/test_fbgemm_fp8.py -k test_bmm python test/dtypes/test_fbgemm_int4.py -k test_bmm Reviewers: Subscribers: Tasks: Tags:
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/2337
Note: Links to docs will display an error until the docs builds have been completed. ❌ 1 New FailureAs of commit bf1b017 with merge base 4235837 ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
@@ -88,6 +89,12 @@ def from_float( | |||
dtype=torch.float, | |||
device=w.device, | |||
) | |||
if transpose_input: | |||
if w.ndim == 3: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe just w.transpose(-1, -2)
|
||
# not used | ||
num_tokens = torch.empty([input_tensor.size(0)], device=input_tensor.device) | ||
xq, x_scale = torch.ops.fbgemm.quantize_fp8_per_row( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ot use num_tokens feels weird, maybe make an issue on fbgemm? or update the op to not need
): | ||
assert len(block_size) == w.ndim, ( | ||
f"Expecting the length of block_size to be equal to the dimension of the weight, got {block_size=} and {w.ndim=}" | ||
) | ||
if int4_row_quantize_zp is None: | ||
raise ImportError("Requires fbgemm-gpu-genai >= 1.2.0") | ||
|
||
if transpose_input: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto here
args[0], | ||
args[1], | ||
) | ||
if not input_tensor.is_floating_point(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is this guard needed? Like is this a common situation to run into
Summary:
att, this PR adds support for running quantized bmm, the quantized bmm kernel for int4 and fp8 (with dynamic activation quantization) requires transpose of weights in order to run, so added transpose_input to the convert function to transpose the weights first
Test Plan:
python test/dtypes/test_fbgemm_fp8.py -k test_bmm
python test/dtypes/test_fbgemm_int4.py -k test_bmm
Reviewers:
Subscribers:
Tasks:
Tags: