[CI] Fix and enable compile/test_config.py in CI #28008
Conversation
- Deleted test_use_cudagraphs_dynamic as use_cudagraph flag is deprecated and its default value (True) is donflicting with cudagraph_mode default value (None) - Updated number of cudagraph captured count in test_use_cudagraphs - Updated test_splitting_ops_dynamic to follow latest behavior of cuda graph: enable_attn_fusion is incompatible with piecewise cudagraphs, splitting_ops is set to empty list, and cudagraph_mode is set to FULL. - Fixed test_cudagraph_sizes_post_init. It used to not respect max_num_seqs and had incorrect assumption about max_num_batched_tokens. Specifically, max_num_batched_tokens should not override explicitly passed cudagraph_capture_sizes. - Improved test_cudagraph_sizes_post_init to take an additional dedicated argument representing expected exceptions to make the test clearer. Signed-off-by: Yanan Cao <[email protected]>
![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 fixes and enables compile/test_config.py in the CI. The changes include removing a deprecated test, updating test values to match current behavior, and significantly refactoring test_cudagraph_sizes_post_init for clarity and correctness. The refactoring correctly addresses an issue where the test had a wrong assumption about how max_num_batched_tokens affects cudagraph_capture_sizes. Overall, the changes are a good improvement to the test suite. I have one comment regarding the expected exception types in the refactored test.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@BoyuanFeng Would like your eyes on the cudagraph config test I fixed, especially I deleted the case about max cudagraph size capped by max_num_batched_tokens. The behavior expected by test seems to contradict current implementation. Thanks @zou3519 Could you take a look so that I can verify the tests with complete CI runs? Thanks. |
|
There is a fix in #27593 |
|
Thank you for the PR! Unfortunately I'm going to have to close it as a duplicate. The PR linked by @BoyuanFeng includes some other related fixes that were exposed by running the tests in this test file so we should proceed with the original PR for a more complete fix. |
3 participants
Signed off by: Yanan Cao [email protected]