-
Notifications
You must be signed in to change notification settings - Fork 78
Fix assertion error on gemm_splitk_benchmark.py #2717
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
| [512, 32768, 8192], | ||
| [1024, 28672, 8192], | ||
| [3072, 4096, 3072], | ||
| [4096, 4096, 4096], |
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.
Adding a new combination breaks the CI and seems a bit out of topic for this pull request. Maybe we should move this change to a separate pull request?
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.
Agree. Split that change in a separate PR please @LiyangLingIntel
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.
Other than that 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.
I disagree, as this PR supposes to fix the 4k functional error.
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've added 4k shape to XeTLA splitk list, benchmark CI works now.
| [512, 32768, 8192], | ||
| [1024, 28672, 8192], | ||
| [3072, 4096, 3072], | ||
| [4096, 4096, 4096], |
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.
Other than that LGTM.
whitneywhtsang
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.
please add gemm_splitk_shape_4096_4096_4096 to XeTLA to fix the CI failure.
The cause of this error is that we should initialize the results tensor when using
atomic_add, otherwise it would read dirty memory from previous benchmarking cases.