Skip to content

[NPU] fix default qwen3-4b script#364

Merged
momo609 merged 2 commits into
vllm-project:ascendfrom
floatlibai:fix/4b-script
Jul 22, 2026
Merged

[NPU] fix default qwen3-4b script#364
momo609 merged 2 commits into
vllm-project:ascendfrom
floatlibai:fix/4b-script

Conversation

@floatlibai

Copy link
Copy Markdown
Contributor

This pr mainly does following:

  • Fix GPU allocation failures caused by NPU_COUNT
  • Correct the invalid PYTHONPATH
  • Simplify the qwen3-4b script

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies the environment setup, cleanup, and Ray cluster startup in the run-qwen3-4B-npu.sh script. Feedback on these changes highlights several critical issues: removing Ray isolation (custom ports and temp directories) can lead to port conflicts and permission errors in shared environments; omitting --runtime-env-json prevents essential environment variables from propagating to Ray workers; and using broad pkill commands for Python and Redis risks terminating unrelated processes. It is recommended to restore the isolated Ray configuration, propagate the runtime environment, and target specific processes during cleanup.

Comment thread scripts/run-qwen3-4B-npu.sh Outdated
Comment on lines +113 to +115
ray start --head --node-ip-address ${MASTER_ADDR} --disable-usage-stats --dashboard-host=0.0.0.0 --dashboard-port=8265

ray job submit --address="http://127.0.0.1:${RAY_DASHBOARD_PORT}" \
--runtime-env-json="${RUNTIME_ENV_JSON}" \
--working-dir="/root/vime" \
ray job submit --address="http://127.0.0.1:8265" \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Removing Ray isolation (custom ports and unique --temp-dir) can cause severe issues in shared or multi-user environments:\n\n1. Port Conflicts: Hardcoding the default dashboard port 8265 and default Ray port 6379 will cause the script to fail if another Ray cluster is already running on the machine.\n2. Permission Denied on /tmp/ray: Without a unique --temp-dir, Ray defaults to /tmp/ray. If another user has already run Ray on this machine, the directory will be owned by them, causing permission errors for other users.\n\nConsider restoring the isolated ports and temp directory configuration.

Comment on lines +115 to 116
ray job submit --address="http://127.0.0.1:8265" \
-- python3 train.py \

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Removing --runtime-env-json from ray job submit prevents critical environment variables (such as PYTHONPATH, LD_LIBRARY_PATH, and HCCL/NPU configurations) from being propagated to the Ray worker processes running the training actors.\n\nRay tasks and actors do not automatically inherit the environment of the shell that submitted the job. Without these variables in the runtime environment, the workers may fail to import required modules or fail to initialize the NPU/HCCL runtime.\n\nPlease restore the --runtime-env-json parameter to ensure the workers have the correct environment.

Comment on lines +8 to +13
pkill -9 ray
pkill -9 python
sleep 3
pkill -9 ray
pkill -9 python
pkill -9 redis

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using pkill -9 python and pkill -9 redis is highly discouraged as it indiscriminately terminates all Python and Redis processes owned by the current user on the machine. This can disrupt other concurrent jobs, system services, or even the CI/CD runner/wrapper executing this script. Consider targeting only the specific training processes (e.g., train.py).

Suggested change
pkill -9 ray
pkill -9 python
sleep 3
pkill -9 ray
pkill -9 python
pkill -9 redis
pkill -9 -f train.py || true

Signed-off-by: flb <floatlibai@gmail.com>
Signed-off-by: flb <floatlibai@gmail.com>
@floatlibai

Copy link
Copy Markdown
Contributor Author

Test result (keep the same parameters as #210):
image

@Meihan-chen

Copy link
Copy Markdown
Collaborator

LGTM

@momo609
momo609 merged commit 45921d4 into vllm-project:ascend Jul 22, 2026
5 checks passed
@floatlibai
floatlibai deleted the fix/4b-script branch July 23, 2026 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants