-
Notifications
You must be signed in to change notification settings - Fork 62
[CI][benchmarks] Option to run all liger tests #3917
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
base: main
Are you sure you want to change the base?
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.
See my two comments
@@ -89,11 +93,21 @@ jobs: | |||
- name: Run Liger-Kernel tests | |||
if: ${{ steps.install.outcome == 'success' && !cancelled() }} | |||
run: | | |||
pip install transformers pandas pytest | |||
pip install transformers pandas pytest pillow |
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.
Let's remove this line, direct pytest call below and do a proper integration with test-triton.sh
like it is done here:
-e 's/,\s*pytest\.mark\.skipif(device="xpu", reason="skip for XPU")//g' \ | ||
-e 's/pytest\.mark\.skipif(device="xpu", reason="skip for XPU"),\s*//g' \ | ||
{} + | ||
fi |
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.
Let's use the same unskip approach as we do for Triton UT's:
TEST_UNSKIP: ${{ inputs.enable_unskip }} if os.getenv('TEST_UNSKIP') == 'true':
@Egor-Krivov ping. Can you address the review comments at your earlies convenience pls? |
Sure. I'm out of office right now and will only be able to get back to this issue on the 6th of May. |
Pull request was converted to draft
Add option to runn all tests from Liger-Kernels, including marked for skipping. Useful to check all cases.
For #3237