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

Feat: Added requantize function and tests #171

Merged
merged 5 commits into from
Apr 18, 2024

Conversation

calmitchell617
Copy link
Contributor

What does this PR do?

This PR addresses issue #162 by adding a requantize function with the following signature, and appropriate tests.

def requantize(model, state_dict):

The function does the following, inplace:

  • Checks to make sure the model isn't a Transformers model which has been distributed via accelerate (ie device_map=auto)
  • Moves the model to the meta device
  • Quantizes it
  • Moves the empty, quantized model to the cpu
  • Loads the state dict into the model.

At this point, the user can move the requantized model to their preferred device.

Who can review?

@dacorvo

Also, FYI @ManoBharathi93

@calmitchell617 calmitchell617 requested a review from dacorvo as a code owner April 12, 2024 15:41
test/model/test_quantize_mlp.py Outdated Show resolved Hide resolved
test/model/test_quantize_mlp.py Outdated Show resolved Hide resolved
@calmitchell617
Copy link
Contributor Author

Added a new commit creating a new file for the tests. I'm not sure what to do RE:

Here we should also check for weight qtype and parameters. Same for activations.

I am happy to try again with more instruction, or just observe how you would do it with a commit of your own.

Copy link
Collaborator

@dacorvo dacorvo left a comment

Choose a reason for hiding this comment

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

Don't forget to use conventional commits or the CI will fail.
You can for instance add all commits during review as:

review: whatever I was asked to do by the almighty reviewer

Also try to check style locally before pushing:

$ make style

@calmitchell617
Copy link
Contributor Author

Ok, I will keep working on it tomorrow. Thanks for your attention on this.

@ManoBharathi93
Copy link

@calmitchell617 since you have not done any commits last 2 days I made PR by referring your commit with @dacorvo suggestions on your commit.

@calmitchell617
Copy link
Contributor Author

Hey @ManoBharathi93, appreciate your persistence. I'm going to continue here anyway, as an exercise.

@calmitchell617
Copy link
Contributor Author

@dacorvo, I believe the requested changes have been made.

@ManoBharathi93
Copy link

Hey @ManoBharathi93, appreciate your persistence. I'm going to continue here anyway, as an exercise.

Looking forward to learn from you @calmitchell617

Copy link
Collaborator

@dacorvo dacorvo left a comment

Choose a reason for hiding this comment

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

Thanks for the pull-request ! I would require a few changes related to the device placement.

quanto/quantize.py Outdated Show resolved Hide resolved
quanto/quantize.py Show resolved Hide resolved
@calmitchell617
Copy link
Contributor Author

Ok, I will make those changes soon.

FYI, I'm writing a review / usage guide of Quanto for my blog. If you would like to see a draft of it to make sure I'm not misrepresenting anything, I would be happy to send you a draft and give you a chance to critique.

@dacorvo
Copy link
Collaborator

dacorvo commented Apr 16, 2024

Ok, I will make those changes soon.

FYI, I'm writing a review / usage guide of Quanto for my blog. If you would like to see a draft of it to make sure I'm not misrepresenting anything, I would be happy to send you a draft and give you a chance to critique.

That's very nice of you to ask. I will be happy to take a look.

@calmitchell617 calmitchell617 requested a review from dacorvo April 18, 2024 08:30
dacorvo
dacorvo previously approved these changes Apr 18, 2024
Copy link
Collaborator

@dacorvo dacorvo left a comment

Choose a reason for hiding this comment

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

Thank you very much for this pull-request !

@dacorvo
Copy link
Collaborator

dacorvo commented Apr 18, 2024

There seems to be an issue with the formatting of your changes. If you applied style locally and don't have that error, please let me know and I will check if we need to update or pin the linter version.

@calmitchell617
Copy link
Contributor Author

Ah, darn it. Forgot to run make style. I'm too used to working on repos by myself :-D

I amended the commit.

@dacorvo dacorvo merged commit 544981d into huggingface:main Apr 18, 2024
11 checks passed
@calmitchell617 calmitchell617 deleted the requantize-function branch April 18, 2024 12:54
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.

3 participants