-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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: add simple tests for weights #2092
Conversation
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.
Really nice to have these tests!
Column gptq/marlin/exl2 and column-packed gptq/marlin tests seem to be missing?
), | ||
}, | ||
"test_get_multi_weights_row_gptq": { | ||
"weight.qweight": torch.tensor( |
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 the quantizers should probably be tested with the real data types to check that there are no accidental dtype conversions happening. So int32
for qweight
, qzeros
, and g_idx
, and f16
for scales
.
Maybe also real shapes? (e.g. currently qzeros
does not line up with just having one group) Probably less important.
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.
great points, I've updated the types in the latest changes but the values are still not fully realistic, happy to follow up in this or a future PR
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.
also I'm not 100% sure if col_packed_exl2
is suppose to be supported or not? I was a bit confused about the expected weight names ("weights" vs "qweights")... is this intended? I'll update the test accordingly 🙂
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.
Great question! This path is missing in weights
, I'll create a ticket and look it. I can update the test here once it's properly implemented.
updated to include more test (21)
|
* feat: add simple tests for weights * fix: adjust types and add tests * fix: adjust so all tests pass * feat: improve weight tests * fix: add missing tests and renames * fix: tweak shapes
This PR adds a few more server tests for loading weights