-
Notifications
You must be signed in to change notification settings - Fork 84
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
Bdellabe/e2e lmeval tests multimodal #1150
base: main
Are you sure you want to change the base?
Conversation
👋 Hi! Thank you for contributing to llm-compressor. Please add the ready label when the PR is ready for review. Note: This is required to complete the testing suite, please only add the label once the PR is code complete and local testing has been performed. |
24c757e
to
a4434cd
Compare
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.
Looks good so far
05e6c32
to
646e329
Compare
Signed-off-by: Brian Dellabetta <[email protected]>
646e329
to
20c6155
Compare
Signed-off-by: Brian Dellabetta <[email protected]>
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.
LGTM!
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.
So good
Signed-off-by: Brian Dellabetta <[email protected]>
7b185dc
SUMMARY:
In order to add multimodal tests to lm_eval, I had to expand the data model and moved some hard-coded logic into conditionals.
This includes a folder re-org, so that lm-eval tests were moved out of
tests/e2e
and into their owntests/lmeval
folder (they are integration+regression tests).We'll have to make sure QA team is aware of the re-org once this lands, as they might have to change the github ci/cd accordingly. This adds roughly 35 minutes of testing to the weekly tests
TEST PLAN:
No new source code