[CI]Recitify Qwen3.8-27B spec decode E2E - #15567
Conversation
Summary of ChangesHello, 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 introduces new end-to-end testing scripts for specific speculative decoding models. The changes focus on rectifying the validation environment for MiniMax-M3-w8a8 and Qwen3.8-27B, ensuring that the speculative decoding mechanisms are correctly configured and verified within the CI pipeline. Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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. Footnotes
|
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
![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
Suggested PR Title:\n\nmarkdown\n[Test][Feature] Add speculative decoding end-to-end tests for MiniMax-M3 and Qwen3.8-27B\n\n\nSuggested PR Summary:\n\nmarkdown\n### What this PR does / why we need it?\nThis pull request adds two new end-to-end speculative decoding tests: `test_eagle3_minimax_m3_w8a8.py` (using 8-way tensor parallelism) and `test_dflash2_qwen38_27b.py` (using 2-way tensor parallelism).\n\nFeedback on the changes highlights two main issues:\n1. Modifying the shared model configuration file (`/mnt/weight/Qwen3.8-27B-DFlash2/config.json`) directly on disk is risky and can lead to permission errors in read-only CI environments or race conditions. It is recommended to use a temporary directory with symlinks instead.\n2. There is a typo in the environment variable `LCCL_DETERMINISTI` (missing the trailing 'C') in both test files, which prevents deterministic execution from being properly enabled.\n\n### Does this PR introduce _any_ user-facing change?\nNo.\n\n### How was this patch tested?\nThis PR adds new end-to-end tests to verify speculative decoding correctness.\n
| draft_model_path = "/mnt/weight/Qwen3.8-27B-DFlash2" | ||
| config_path = os.path.join(draft_model_path, "config.json") | ||
| if os.path.exists(config_path): | ||
| with open(config_path, "r", encoding="utf-8") as f: | ||
| config = json.load(f) | ||
| if "layer_types" in config and isinstance(config["layer_types"], list): | ||
| config["layer_types"] = ["full_attention"] * len(config["layer_types"]) | ||
| config["sliding_window"] = None | ||
| config["use_sliding_window"] = False | ||
| with open(config_path, "w", encoding="utf-8") as f: | ||
| json.dump(config, f, indent=2) | ||
|
|
||
| _run_speculative_decoding( | ||
| model_name=model_name, | ||
| speculative_config={ | ||
| "method": "dflash", | ||
| "model": "UploadWeight/Qwen3.8-27B-DFlash2", | ||
| "num_speculative_tokens": num_speculative_tokens, | ||
| "enforce_eager": True, | ||
| }, | ||
| expected_acceptance_length=expected_acceptance_length, | ||
| runner_kwargs={ | ||
| "tensor_parallel_size": 2, | ||
| "max_model_len": 8096, | ||
| "compilation_config": CompilationConfig(cudagraph_mode="NONE"), # 映射自 compilation-config '{"cudagraph_mode": "NONE"}' | ||
| "additional_config": additional_config, | ||
| }, | ||
| is_moe=False, | ||
| ) No newline at end of file |
There was a problem hiding this comment.
Modifying the shared model configuration file (/mnt/weight/Qwen3.8-27B-DFlash2/config.json) directly on disk is highly risky. In CI environments, /mnt/weight is often mounted as read-only, which will cause this test to fail with a permission error. Even if writable, modifying it in-place can cause race conditions if other tests run in parallel, or leave persistent side effects that break subsequent tests.\n\nTo prevent this, create a temporary directory, symlink the weight files from the original directory, write the modified config.json there, and pass the temporary directory path as the draft model.
import tempfile\n\n draft_model_path = "/mnt/weight/Qwen3.8-27B-DFlash2"\n config_path = os.path.join(draft_model_path, "config.json")\n\n with tempfile.TemporaryDirectory() as tmp_dir:\n tmp_draft_model_path = tmp_dir\n if os.path.exists(config_path):\n with open(config_path, "r", encoding="utf-8") as f:\n config = json.load(f)\n if "layer_types" in config and isinstance(config["layer_types"], list):\n config["layer_types"] = ["full_attention"] * len(config["layer_types"])\n config["sliding_window"] = None\n config["use_sliding_window"] = False\n\n for item in os.listdir(draft_model_path):\n if item != "config.json":\n os.symlink(os.path.join(draft_model_path, item), os.path.join(tmp_draft_model_path, item))\n\n with open(os.path.join(tmp_draft_model_path, "config.json"), "w", encoding="utf-8") as f:\n json.dump(config, f, indent=2)\n else:\n tmp_draft_model_path = "UploadWeight/Qwen3.8-27B-DFlash2"\n\n _run_speculative_decoding(\n model_name=model_name,\n speculative_config={\n "method": "dflash",\n "model": tmp_draft_model_path,\n "num_speculative_tokens": num_speculative_tokens,\n "enforce_eager": True,\n },\n expected_acceptance_length=expected_acceptance_length,\n runner_kwargs={\n "tensor_parallel_size": 2,\n "max_model_len": 8096,\n "compilation_config": CompilationConfig(cudagraph_mode="NONE"),\n "additional_config": additional_config,\n },\n is_moe=False,\n )| "TASK_QUEUE_ENABLE": "1", | ||
| "VLLM_LOGGING_LEVEL": "INFO", | ||
| "VLLM_USE_V2_MODEL_RUNNER": "1", | ||
| "LCCL_DETERMINISTI": "1", |
There was a problem hiding this comment.
There is a typo in the environment variable name: LCCL_DETERMINISTI should be LCCL_DETERMINISTIC. Without the trailing 'C', this environment variable will be silently ignored, and deterministic execution for LCCL will not be enabled, potentially leading to non-deterministic test failures.
| "LCCL_DETERMINISTI": "1", | |
| "LCCL_DETERMINISTIC": "1", |
There was a problem hiding this comment.
This parameter is correct.
| "HCCL_BUFFSIZE": "1024", | ||
| "TASK_QUEUE_ENABLE": "1", | ||
| "HCCL_OP_EXPANSION_MODE": "AIV", | ||
| "LCCL_DETERMINISTI": "1", |
There was a problem hiding this comment.
There is a typo in the environment variable name: LCCL_DETERMINISTI should be LCCL_DETERMINISTIC. Without the trailing 'C', this environment variable will be silently ignored, and deterministic execution for LCCL will not be enabled, potentially leading to non-deterministic test failures.
| "LCCL_DETERMINISTI": "1", | |
| "LCCL_DETERMINISTIC": "1", |
There was a problem hiding this comment.
This parameter is correct.
Signed-off-by: rainney <1345096680@qq.com>
Signed-off-by: rainney <1345096680@qq.com>
Signed-off-by: rainney <1345096680@qq.com>
Signed-off-by: rainney <1345096680@qq.com>
b32980f to
55b1ac5
Compare
sunny-rain-63
changed the title
1 participant
What this PR does / why we need it?
Rectify the E2E tests for Qwen3.8-27B speculative decoding to ensure accurate validation.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing CI tests all pass, including the rectified E2E tests for Qwen3.8-27B speculative decoding.