This repository was archived by the owner on Sep 10, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 248
Implement the AO API in torchchat quantization handlers and unify logic. #1291
Merged
Merged
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
8b66b1b
Align AO and CHAT quantizers in quantize.py
mikekgfb 4378b26
Typo / Docs
mikekgfb 4cbd6b6
Merge branch 'main' into patch-2
mikekgfb be06ff4
Update quantize.py
mikekgfb 0b62eb7
Merge branch 'main' into patch-2
Jack-Khuu bb3c3cd
Update quantize.py
mikekgfb 5f19018
Fix typo
mikekgfb 3956f53
Fix default args
mikekgfb 72c1373
Update quantize.py
mikekgfb 8b924d9
Update quantize.py
mikekgfb 3168bc7
Update quantize.py
mikekgfb 159597c
Merge branch 'main' into patch-2
mikekgfb 6926b9c
Merge branch 'main' into patch-2
mikekgfb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Any reason we can't put this inside of the
quantizer_class_dict?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.
It seems cleaner to try import
Int8DynActIntxWeightQuantizerfirst and fallback toErrorHandler, then assign this handler tolinear:a8wxdqinquantizer_class_dict.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 is what the code is doing now. Try to import, and if the import fails, set up the error handler.
Or were you thinking to do a wrapper that imports the class
Int8DynActIntxWeightQuantizerand then calls the error if it fails, and the imported method if the import succeeds? I assumed that all the conditional import will go away soonish since we should know what version of AO we pin, and whether it has the new class. (And that enablement is there, it doesn't disappear again, so that we can just do a simple import in the future.).... Because init can't return an alternate class, the wrapper would have to redispatch all methods internally, which isn't a big deal per se, but may add readability concerns? I'm happy to go either way, ideally as a follow-on.
LMK what you think the best long-term trajectory is for this functionality, and we'll align the code to that. (I think the current version is preferable to the previous state, b/c we don't have to special case in the dispatch loop)
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.
I think if this can go away soon I don't have a strong opinion and can live with this.