-
Notifications
You must be signed in to change notification settings - Fork 66
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
Conversation
Added a new commit creating a new file for the tests. I'm not sure what to do RE:
I am happy to try again with more instruction, or just observe how you would do it with a commit of your own. |
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.
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
Ok, I will keep working on it tomorrow. Thanks for your attention on this. |
@calmitchell617 since you have not done any commits last 2 days I made PR by referring your commit with @dacorvo suggestions on your commit. |
Hey @ManoBharathi93, appreciate your persistence. I'm going to continue here anyway, as an exercise. |
…tested qtype equivalence
e82a862
to
1da62f2
Compare
@dacorvo, I believe the requested changes have been made. |
Looking forward to learn from you @calmitchell617 |
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.
Thanks for the pull-request ! I would require a few changes related to the device placement.
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. |
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.
Thank you very much for this pull-request !
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. |
…nal device after requantizing.
03b20f3
to
5c33ba9
Compare
Ah, darn it. Forgot to run I amended the commit. |
What does this PR do?
This PR addresses issue #162 by adding a requantize function with the following signature, and appropriate tests.
The function does the following, inplace:
device_map=auto
)At this point, the user can move the requantized model to their preferred device.
Who can review?
@dacorvo
Also, FYI @ManoBharathi93