Skip to content

Conversation

@vkuzo
Copy link
Contributor

@vkuzo vkuzo commented Nov 3, 2025

Summary:

Change the granularity for this scaling type from
PerBlock((1, 128)), PerBlock((128, 128)) to PerBlock([1, 128]), PerBlock([128, 128]),
to get around the current limitation of config serialization not
supporting tuples.

Error when serializing tuples:
https://gist.github.com/vkuzo/ab4d6aec83cb98ad9417898d2c024a2c

Supporting tuples and lists would be a better long term fix, but the
workaround will unblock further work in the short term.

Test Plan:

pytest test/quantization/quantize_/workflows/float8/test_float8_tensor.py -s -x

Reviewers:

Subscribers:

Tasks:

Tags:

[ghstack-poisoned]
@vkuzo
Copy link
Contributor Author

vkuzo commented Nov 3, 2025

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 3, 2025

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit e6b16a3 with merge base f856d36 (image):
💚 Looks good so far! There are no failures yet. 💚

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

vkuzo added a commit that referenced this pull request Nov 3, 2025
Summary:

Change the granularity for this scaling type from
`PerBlock((1, 128)), PerBlock((128, 128))` to `PerBlock([1, 128]), PerBlock([128, 128])`,
to get around the current limitation of config serialization not
supporting tuples.

Error when serializing tuples:
https://gist.github.com/vkuzo/ab4d6aec83cb98ad9417898d2c024a2c

Supporting tuples and lists would be a better long term fix, but the
workaround will unblock further work in the short term.

Test Plan:

```
pytest test/quantization/quantize_/workflows/float8/test_float8_tensor.py -s -x
```

Reviewers:

Subscribers:

Tasks:

Tags:
ghstack-source-id: e150e2a
ghstack-comment-id: 3480842282
Pull-Request: #3279
@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 Nov 3, 2025
@vkuzo vkuzo added the topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories) label Nov 3, 2025
[ghstack-poisoned]
vkuzo added a commit that referenced this pull request Nov 3, 2025
Summary:

Change the granularity for this scaling type from
`PerBlock((1, 128)), PerBlock((128, 128))` to `PerBlock([1, 128]), PerBlock([128, 128])`,
to get around the current limitation of config serialization not
supporting tuples.

Error when serializing tuples:
https://gist.github.com/vkuzo/ab4d6aec83cb98ad9417898d2c024a2c

Supporting tuples and lists would be a better long term fix, but the
workaround will unblock further work in the short term.

Test Plan:

```
pytest test/quantization/quantize_/workflows/float8/test_float8_tensor.py -s -x
```

Reviewers:

Subscribers:

Tasks:

Tags:
ghstack-source-id: 68799cc
ghstack-comment-id: 3480842282
Pull-Request: #3279
KernelPreference.AUTO,
KernelPreference.TORCH,
), "unimplemented"
assert self.mm_config is None, "unimplemented"
Copy link
Contributor

Choose a reason for hiding this comment

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

is this supported now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, this is tested with the serialization flow. When the object is created a default value of mm_config is set, and then when object is saved and reloaded, that value then makes it to the constructor.

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. topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants