Skip to content

Conversation

@cyxlily
Copy link
Contributor

@cyxlily cyxlily commented Dec 22, 2025

Based on #3468
Separately control the activation quantization granularity and weight quantization granularity

@pytorch-bot
Copy link

pytorch-bot bot commented Dec 22, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/3524

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 22, 2025
@cyxlily cyxlily marked this pull request as draft December 22, 2025 09:31
else:
raise ValueError(f"Unexpected step: {step}")

if isinstance(base_config, Int8StaticActivationInt8WeightConfig):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think smoothquant should not mention specific configs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, updated.


scale: torch.Tensor
granularity: Granularity = PerRow()
act_granularity: Granularity = PerTensor()
Copy link
Contributor

Choose a reason for hiding this comment

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

also we just use tuple, like this:

granularity: Optional[Union[FP8Granularity, List[FP8Granularity]]] = None

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, updated.

Signed-off-by: Cui, Lily <[email protected]>
):
assert config.granularity in {PerRow(), PerTensor()}, (
"Only PerRow and PerTensor is supported currently"
assert isinstance(config.granularity, (tuple, list)) and len(config.granularity) == 2, (
Copy link
Collaborator

Choose a reason for hiding this comment

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

According to the code above, it can only be a list, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the comment, updated.

if isinstance(base_config, Int8StaticActivationInt8WeightConfig):
quantize_(
basic_model,
Int8DynamicActivationInt8WeightConfig(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Dynamic or static?

Copy link
Contributor Author

@cyxlily cyxlily Dec 24, 2025

Choose a reason for hiding this comment

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

When testing static, It uses dynamic as the basic model. Otherwise, directly quantizing the static config as the basic config will result in errors due to missing act_scale computed by static_scale.

Signed-off-by: Cui, Lily <[email protected]>
Signed-off-by: Cui, Lily <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants