[Refactor][Config] Remove weight NZ environment variable - #14710
[Refactor][Config] Remove weight NZ environment variable#14710kk-ss1999 wants to merge 1 commit into
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 completes the migration of the 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
|
Use additional_config.weight_nz_mode as the only configuration source and migrate runtime consumers, deployment examples, tests, and documentation. Signed-off-by: kk_1999 <97623903+kk-ss1999@users.noreply.github.com>
10a45c2 to
405023a
Compare
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Ops][Misc] Remove legacy VLLM_ASCEND_ENABLE_NZ environment variable and migrate to weight_nz_modeSuggested PR Summary:
### What this PR does / why we need it?
This PR removes the legacy `VLLM_ASCEND_ENABLE_NZ` environment variable and fully transitions to using `weight_nz_mode` within `additional_config` for configuring the weight NZ mode on Ascend NPU. It updates documentation, examples, end-to-end tests, and unit tests to reflect this change.
Feedback: In `tests/e2e/weekly/single_node/models/test_qwen3_30b_acc.py`, the removal of `VLLM_ASCEND_ENABLE_NZ="2"` was not migrated to `weight_nz_mode: 2` in the server arguments, which may cause accuracy degradation or test failures.
### Does this PR introduce _any_ user-facing change?
Yes, the legacy `VLLM_ASCEND_ENABLE_NZ` environment variable is no longer supported. Users must now use `additional_config.weight_nz_mode` (with values `0` to disable NZ, `1` to enable NZ only for quantized weights, and `2` to enable NZ for BF16/FP16 weights when supported).
### How was this patch tested?
The changes were tested by updating existing unit tests and various end-to-end test configurations.| "HCCL_BUFFSIZE": "1024", | ||
| "OMP_NUM_THREADS": "1", | ||
| "PYTORCH_NPU_ALLOC_CONF": "expandable_segments:True", | ||
| "VLLM_ASCEND_ENABLE_NZ": "2", |
There was a problem hiding this comment.
The environment variable VLLM_ASCEND_ENABLE_NZ was set to "2" in this test, but it has been removed without migrating the configuration to --additional-config '{"weight_nz_mode": 2}' in server_args. This will cause the test to run with the default weight_nz_mode of 1, which might lead to accuracy degradation or test failures for the Qwen3-30B-A3B-W8A8 model. Please ensure "weight_nz_mode": 2 is added to the --additional-config argument in server_args.
|
👋 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. |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
What this PR does / why we need it?
Removes the legacy
VLLM_ASCEND_ENABLE_NZenvironment variable after its behavior was migrated toadditional_config.weight_nz_mode. Runtime code, deployment examples, test configurations, and documentation are migrated to the config field where applicable.This is one independently reviewable part of the seven-variable split of #14637.
Does this PR introduce any user-facing change?
Yes.
VLLM_ASCEND_ENABLE_NZis no longer read. Users must configure the behavior with--additional-config '{"weight_nz_mode":1}'.How was this patch tested?
ruff checkandruff format --checkon changed Python filesPython byte-compilation of changed Python files
YAML parsing of changed YAML files
JSON parsing of concrete
additional-configpayloadsScan confirming no active references to
VLLM_ASCEND_ENABLE_NZremain outside migration documentation and the ignored-variable unit testNPU end-to-end tests were not run in this Windows environment
vLLM version: v0.27.1
vLLM main: vllm-project/vllm@58d3918