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

[Observer Restructure]: Separate out scale/zp and observer init; separate out calibration from forward pass #188

Merged
merged 15 commits into from
Oct 18, 2024

Conversation

dsikka
Copy link
Contributor

@dsikka dsikka commented Oct 8, 2024

Summary

  • Main goal of this PR is to separate out observer functionality which has been intertwined with other steps within compressed-tensors, specifically calibration/forward pass
  • The purpose of this PR is to work as a stepping stone PR to move the observer code from compressed-tensors
  • These changes include:
  1. Separate out the observer and zp/scale initialization; scale and zp initialization will remain as is while observer initialization will move out of compressed-tensors
  2. Separate calibration from the forward pass

Testing:

@dsikka dsikka changed the title [Observer Restructure] Separate out scale/zp and observer init [Observer Restructure] Separate out scale/zp and observer init; separate calibration and forward pass Oct 9, 2024
@dsikka dsikka changed the title [Observer Restructure] Separate out scale/zp and observer init; separate calibration and forward pass [Observer Restructure] Separate out scale/zp and observer init; separate out calibration from forward pass Oct 9, 2024
@dsikka dsikka changed the title [Observer Restructure] Separate out scale/zp and observer init; separate out calibration from forward pass [Observer Restructure]: Separate out scale/zp and observer init; separate out calibration from forward pass Oct 9, 2024
@dsikka dsikka marked this pull request as ready for review October 13, 2024 17:10
@markurtz markurtz self-requested a review October 14, 2024 13:34
@markurtz
Copy link
Member

@dsikka double checking, this is ready for review now?

@dsikka
Copy link
Contributor Author

dsikka commented Oct 14, 2024

@dsikka double checking, this is ready for review now?

Yup

kylesayrs
kylesayrs previously approved these changes Oct 14, 2024
Copy link
Contributor

@kylesayrs kylesayrs left a comment

Choose a reason for hiding this comment

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

So much clearer

src/compressed_tensors/quantization/lifecycle/forward.py Outdated Show resolved Hide resolved
rahul-tuli
rahul-tuli previously approved these changes Oct 16, 2024
Copy link
Member

@rahul-tuli rahul-tuli left a comment

Choose a reason for hiding this comment

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

LGTM! with minor nits

@dsikka dsikka dismissed stale reviews from rahul-tuli and kylesayrs via 38fe6f6 October 18, 2024 00:15
@dsikka dsikka requested a review from kylesayrs October 18, 2024 00:20
@dsikka dsikka requested a review from rahul-tuli October 18, 2024 01:41
@dsikka dsikka merged commit 955e906 into main Oct 18, 2024
1 check passed
@dsikka dsikka deleted the update-init branch October 18, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants