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.

Sign up for GitHub

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

Jump to bottom

[WIP] GPTQ MultiTensor refactor #716

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

[WIP] GPTQ MultiTensor refactor #716

wants to merge 13 commits into from

Conversation

danielpatrickhug
Copy link
Contributor

This is a PR in response to issue #577
The primary contributions of this PR are as follows:

  1. Refactor the GPTQ module to use the MultiTensor class described here instead of the GenericGPTQRunner class which subclasses the fx.Interpreter.
  2. Refactor the GPTQQuantizer class and the Int4WeightOnlyGPTQQuantizer class which previously depended on the GenericGPTQRunner class.

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Aug 20, 2024

🔗 Helpful Links

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

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.

@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 Aug 20, 2024
@danielpatrickhug danielpatrickhug marked this pull request as draft August 21, 2024 14:20
@jerryzh168
Copy link
Contributor

@danielpatrickhug thanks for working on this, could you follow the flow described in #721 for the refactor? we want to deprecate the "Quantizer" based APIs

jerryzh168
jerryzh168 reviewed Aug 21, 2024
View reviewed changes
torchao/quantization/GPTQ_MT.py
total_batches = 0

outputs = []
with torch._C.DisableTorchFunctionSubclass():
Copy link
Contributor

Choose a reason for hiding this comment

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

this part can also be decoupled from the MultiTensor I feel

@danielpatrickhug
Copy link
Contributor Author

danielpatrickhug commented Aug 22, 2024 •
edited
Loading

@danielpatrickhug thanks for working on this, could you follow the flow described in #721 for the refactor? we want to deprecate the "Quantizer" based APIs

Sure no problem, I'll work on adapting it to this API format.

@msaroufim msaroufim requested a review from HDCharles August 22, 2024 18:00
@HDCharles
Copy link
Contributor

@danielpatrickhug thanks for working on this, could you follow the flow described in #721 for the refactor? we want to deprecate the "Quantizer" based APIs

i don't think we should combine a functional refactor with an API refactor. This should be a drop in replacement for what we had, otherwise we should make those API changes first and let OSS devs make their changes on top. I can do the API alignment after the functional refactor is complete.

yanbing-j pushed a commit to yanbing-j/ao that referenced this pull request Dec 9, 2024
@mikekgfb @malfet
Handle MPS with for export and generate+compile (pytorch#716)
e503d7a 
* Handle compile for export and generate

* typo

* typo

* typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jerryzh168 jerryzh168 jerryzh168 left review comments

@HDCharles HDCharles Awaiting requested review from HDCharles

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danielpatrickhug @jerryzh168 @HDCharles @facebook-github-bot