Skip to content
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

Move fpx to tensor subclass #1603

Merged
merged 4 commits into from
Jan 24, 2025
Merged

Move fpx to tensor subclass #1603

merged 4 commits into from
Jan 24, 2025

Conversation

jainapurva
Copy link
Contributor

@jainapurva jainapurva commented Jan 23, 2025

Make a new FpxTensor subclass, for fpx (float1-float7) quantizations.

Motivation: Fpx, has it's own implementation of choose_qparams_and_quantize_affine_fpx, quantization and dequantization for floatx quantization, hence a new tensor-subclass improves the code-quality. It reduces re-directions in AQT, and improves readability.

Future goals: Remove the AQT abstraction and make it independent tensor-subclass

Test Plan : Green CI

Copy link

pytorch-bot bot commented Jan 23, 2025

🔗 Helpful Links

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

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

⏳ No Failures, 4 Pending

As of commit 77167eb with merge base 0fae693 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot facebook-github-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 Jan 23, 2025
@jainapurva jainapurva added topic: not user facing Use this tag if you don't want this PR to show up in release notes topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories) topic: for developers Use this tag if this PR is mainly developer facing and removed topic: improvement Use this tag if this PR is an improvement (doesn't fit into any of the other categories) labels Jan 23, 2025
@jainapurva jainapurva marked this pull request as ready for review January 23, 2025 17:10
Copy link
Contributor

@danielvegamyhre danielvegamyhre left a comment

Choose a reason for hiding this comment

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

LGTM, left one minor comment

@@ -456,6 +462,53 @@ class FloatxTensorCoreLayout(Layout):
mbits: int


class FloatxTensor(AffineQuantizedTensor):
"""
Floatx quantized tensor subclass which inherits AffineQuantizedTensor class.
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be helpful for new users/developers to add a short sentence to the docstring explaining the difference between floatx and float8.

Copy link
Contributor

@drisspg drisspg left a comment

Choose a reason for hiding this comment

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

Ditto Daniels comment

@jainapurva jainapurva merged commit 70be245 into main Jan 24, 2025
19 checks passed
jainapurva added a commit that referenced this pull request Jan 24, 2025
jainapurva added a commit that referenced this pull request Jan 24, 2025
Revert "Move fpx to tensor subclass (#1603)"

This reverts commit 70be245.
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: for developers Use this tag if this PR is mainly developer facing topic: not user facing Use this tag if you don't want this PR to show up in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants