-
Notifications
You must be signed in to change notification settings - Fork 185
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
Add tests in CI #50
Add tests in CI #50
Conversation
print("lm_eval not available, skip defining GPTQQuantizer") | ||
|
||
|
||
class GPTQQuantizer(Quantizer): |
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.
should this live under if
branch as well?
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 would vote for no, because the eval wrappers are correctly skipped if eleuther harness is not available and I'd rather we seperate algorithm and correctness checks. In some sense though you're right in that this file needs better tests. My goal right now was to make existing tests green so I'd rather we revisit these changes
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.
Agreed, we shouldn't need accuracy checks to run inference, so we shouldn't need lm_eval to use the quantizer.
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.
right now GPTQQuantizer has dependency on lm-eval though, it's not for accuracy eval, it's used to gather the inputs from tasks. not sure how we want to remove the dependency there
* Add tests in CI * fix test * Update regression_test.yml * Update regression_test.yml * Update regression_test.yml * Update regression_test.yml * fix test * test * upd * more reqs * checkpoint * update * changes * push
This just runs all the tests in test on a T4 machine notable changes
Let's merge this ASAP so we can also start doing nightly releases with this #49
Locally
There's still some todos in a later PR