[AMD CI] Add moonshotai/Kimi-K2-Instruct-0905 testcases#17656
[AMD CI] Add moonshotai/Kimi-K2-Instruct-0905 testcases#17656HaiShaw merged 9 commits intosgl-project:mainfrom
Conversation
Summary of ChangesHello @sogalin, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request expands the continuous integration test coverage for AMD GPUs by integrating new test cases for the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new test case for the moonshotai/Kimi-K2-Instruct-0905 model on AMD GPUs, which is a valuable addition for increasing test coverage. The code is well-structured and follows the project's testing conventions. My review includes a few suggestions to improve the robustness of URL parsing within the new test file, making it more maintainable and less prone to breaking from future changes to the test configuration.
| @@ -0,0 +1,95 @@ | |||
| import os | |||
| import unittest | |||
| from types import SimpleNamespace | |||
There was a problem hiding this comment.
To robustly parse the base URL, it's recommended to use Python's built-in urlparse function. Please add the necessary import. This will make the URL handling in the test methods less brittle and more maintainable.
| from types import SimpleNamespace | |
| from types import SimpleNamespace | |
| from urllib.parse import urlparse |
| args = SimpleNamespace( | ||
| num_shots=8, | ||
| data_path=None, | ||
| num_questions=1319, | ||
| parallel=1319, | ||
| max_new_tokens=512, | ||
| host="http://127.0.0.1", | ||
| port=int(self.base_url.split(":")[-1]), | ||
| ) |
There was a problem hiding this comment.
Hardcoding the host URL and manually parsing the port is brittle. For instance, if DEFAULT_URL_FOR_TEST were to use localhost instead of an IP address, this test could fail. Using urlparse (with the import added at the top of the file) to deconstruct self.base_url is a more robust approach.
| args = SimpleNamespace( | |
| num_shots=8, | |
| data_path=None, | |
| num_questions=1319, | |
| parallel=1319, | |
| max_new_tokens=512, | |
| host="http://127.0.0.1", | |
| port=int(self.base_url.split(":")[-1]), | |
| ) | |
| parsed_url = urlparse(self.base_url) | |
| args = SimpleNamespace( | |
| num_shots=8, | |
| data_path=None, | |
| num_questions=1319, | |
| parallel=1319, | |
| max_new_tokens=512, | |
| host=f"{parsed_url.scheme}://{parsed_url.hostname}", | |
| port=parsed_url.port, | |
| ) |
| self.assertGreater(metrics["accuracy"], 0.94) | ||
|
|
||
| def test_bs_1_speed(self): | ||
| args = BenchArgs(port=int(self.base_url.split(":")[-1]), max_new_tokens=2048) |
There was a problem hiding this comment.
Similar to test_a_gsm8k, manually parsing the port from the URL is brittle. Using urlparse provides a more robust way to extract the port and improves maintainability.
| args = BenchArgs(port=int(self.base_url.split(":")[-1]), max_new_tokens=2048) | |
| parsed_url = urlparse(self.base_url) | |
| args = BenchArgs(port=parsed_url.port, max_new_tokens=2048) |
sogalin
requested review from
Fridge003,
Kangyan-Zhou,
ispobock and
merrymercy
as code owners
|
Currently, 2 stage-c test mi35x test maybe the limit due to long queue time (since limited mi35x runner). We may either put dpsk3.2 or kimi mi35x in PR and put the other one in nightly. cc: @yctseng0211 @bingxche |
we will move dpsk3.2 to 325 8-gpu with this PR : #17633 |
4 participants
Motivation
Add kimi_k2_instruct model testcases to increase the test coverage on AMD GPUs.
Change CI job partition from 2 to 3.
Modifications
Add 1 test script : test/registered/amd/test_kimi_k2_instruct.py
Accuracy Tests
Accuracy: 0.956
Invalid: 0.000
Latency: 25.643 s
Output throughput: 5280.565 token/s
Benchmarking and Profiling
+-------------+--------+------------+-----------------+
| Latency (s) | Tokens | Acc Length | Speed (token/s) |
+-------------+--------+------------+-----------------+
| 21.410 | 1001 | 1.000 | 46.75 |
+-------------+--------+------------+-----------------+
speed=46.75
Checklist
Review Process
/tag-run-ci-label,/rerun-failed-ci,/tag-and-rerun-ci