-
Notifications
You must be signed in to change notification settings - Fork 76
Enable python/triton_kernels/tests as optional in test-triton.sh #4924
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
Conversation
|
What else is left to enable in |
Current status is: How Triton uses The CI runs integration tests on NVIDIA and AMD dedicated yamls: |
Shall we install |
python/triton_kernels/tests/test_tensor_details/test_layout_hopper.py
Outdated
Show resolved
Hide resolved
In upstream they don't install it. I was also able to run tests on VM and GH without additional installations. |
python/triton_kernels/tests/test_tensor_details/test_layout_hopper.py
Outdated
Show resolved
Hide resolved
0afea8a to
c545a90
Compare
vlad-penkin
left a comment
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.
Updated PR looks good for me in general.
Let's add minimal support in CI as an optional target and have one green run before final approval.
How do i suggest to track the work on enabling skipped tests - keep the linked issue open and add additional PR or create a new issue?
|
CI is green. I'd suggest to open an issue to track re-enableing of skipped tests. IMO this PR can go in @vlad-penkin. |
|
What's the impact in CI time and pass rate? |
FYI: work on this has already begun #5051. However, I think this pull request should be merged earlier. @vlad-penkin please note that this pull request is mostly waiting for your review. @jakub-sochacki could you fix conflicts please? |
79 new tests, less than 5 minutes of CI time. Can we measure it precisely after merging? |
I don't currently see it in this PR, I assume it will be added. |
@jakub-sochacki --unit)
TEST_UNIT=true
TEST_DEFAULT=false
shift
;;In fact, to enable these tests in CI you also need to add code to - name: Run triton kernels tests
if: matrix.suite == 'rest'
run: |
${{ env.TRITON_TEST_CMD }} --triton-kernelsUPD: the similar code should be added into |
|
Some CI checks run manually:
|
vlad-penkin
left a comment
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
Pass rate: 98.6%->84.11%, CI time increased by ~1min. |
Pass rate will be better after #5051 |
Uh oh!
There was an error while loading. Please reload this page.