[AMD] Run MI355X disaggregation Nightly Test with runtime checkout code mechanism - #30386
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces the SGLANG_USE_CHECKOUT_RUNTIME option to scripts/ci/slurm/launch_mi355x.sh, allowing the Slurm compute-node containers to reinstall and run the sglang package directly from the workflow's git checkout instead of using the image's baked-in version. The review feedback suggests optimizing the workspace staging by excluding the .git directory from the tarball to save disk space and transfer time. Additionally, it recommends using conditional parameter expansion when modifying PYTHONPATH to avoid trailing colons, which can unintentionally append the current working directory to Python's search path.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| tar --exclude='__pycache__' --exclude='*.pyc' \ | ||
| -C "$GITHUB_WORKSPACE" -cf - . | tar -C "$CHECKOUT_STAGE" -xf - |
There was a problem hiding this comment.
Staging the entire $GITHUB_WORKSPACE directory over shared NFS can be slow and consume significant disk space if the .git directory is large. Excluding the .git directory from the tar command is a safe optimization since the checkout SHA is already retrieved via git rev-parse and passed as an environment variable.
| tar --exclude='__pycache__' --exclude='*.pyc' \ | |
| -C "$GITHUB_WORKSPACE" -cf - . | tar -C "$CHECKOUT_STAGE" -xf - | |
| tar --exclude='__pycache__' --exclude='*.pyc' --exclude='.git' \ | |
| -C "$GITHUB_WORKSPACE" -cf - . | tar -C "$CHECKOUT_STAGE" -xf - |
| python3 -m pip install --no-deps --no-build-isolation -e "$RUNTIME_CHECKOUT/python" | ||
|
|
||
| export RUNTIME_CHECKOUT | ||
| export PYTHONPATH="$RUNTIME_CHECKOUT/python:${PYTHONPATH:-}" |
There was a problem hiding this comment.
Using :${PYTHONPATH:-} when PYTHONPATH is empty or unset results in a trailing colon (e.g., /path/to/dir:). In Python, an empty entry in PYTHONPATH (such as a trailing colon) is implicitly treated as the current working directory (.), which can lead to unexpected import behavior or security risks. Using the ${PYTHONPATH:+:$PYTHONPATH} parameter expansion cleanly appends the existing PYTHONPATH only if it is non-empty.
| export PYTHONPATH="$RUNTIME_CHECKOUT/python:${PYTHONPATH:-}" | |
| export PYTHONPATH="$RUNTIME_CHECKOUT/python${PYTHONPATH:+:$PYTHONPATH}" |
| source "\$CIDIR/model_flags.sh" | ||
| bash "\$CIDIR/install_checkout_sglang.sh" | ||
| if [[ "\${SGLANG_USE_CHECKOUT_RUNTIME:-1}" != "0" ]]; then | ||
| export PYTHONPATH=/tmp/sglang-checkout-runtime/python:\${PYTHONPATH:-} |
There was a problem hiding this comment.
Using :\${PYTHONPATH:-} when PYTHONPATH is empty or unset results in a trailing colon, which implicitly adds the current working directory to Python's search path. Use the \${PYTHONPATH:+:\$PYTHONPATH} parameter expansion to safely append the existing PYTHONPATH only if it is non-empty.
| export PYTHONPATH=/tmp/sglang-checkout-runtime/python:\${PYTHONPATH:-} | |
| export PYTHONPATH="/tmp/sglang-checkout-runtime/python\${PYTHONPATH:+:\$PYTHONPATH}" |
| source "\$CIDIR/model_flags.sh" | ||
| bash "\$CIDIR/install_checkout_sglang.sh" | ||
| if [[ "\${SGLANG_USE_CHECKOUT_RUNTIME:-1}" != "0" ]]; then | ||
| export PYTHONPATH=/tmp/sglang-checkout-runtime/python:\${PYTHONPATH:-} |
There was a problem hiding this comment.
Using :\${PYTHONPATH:-} when PYTHONPATH is empty or unset results in a trailing colon, which implicitly adds the current working directory to Python's search path. Use the \${PYTHONPATH:+:\$PYTHONPATH} parameter expansion to safely append the existing PYTHONPATH only if it is non-empty.
| export PYTHONPATH=/tmp/sglang-checkout-runtime/python:\${PYTHONPATH:-} | |
| export PYTHONPATH="/tmp/sglang-checkout-runtime/python\${PYTHONPATH:+:\$PYTHONPATH}" |
| if [ "\${SGLANG_USE_CHECKOUT_RUNTIME:-1}" != "0" ]; then | ||
| export PYTHONPATH=/tmp/sglang-checkout-runtime/python:\${PYTHONPATH:-} | ||
| else | ||
| export PYTHONPATH=/sgl-workspace/sglang/python:\${PYTHONPATH:-} | ||
| fi |
There was a problem hiding this comment.
Using :\${PYTHONPATH:-} when PYTHONPATH is empty or unset results in a trailing colon, which implicitly adds the current working directory to Python's search path. Use the \${PYTHONPATH:+:\$PYTHONPATH} parameter expansion to safely append the existing PYTHONPATH only if it is non-empty.
| if [ "\${SGLANG_USE_CHECKOUT_RUNTIME:-1}" != "0" ]; then | |
| export PYTHONPATH=/tmp/sglang-checkout-runtime/python:\${PYTHONPATH:-} | |
| else | |
| export PYTHONPATH=/sgl-workspace/sglang/python:\${PYTHONPATH:-} | |
| fi | |
| if [ "\${SGLANG_USE_CHECKOUT_RUNTIME:-1}" != "0" ]; then | |
| export PYTHONPATH="/tmp/sglang-checkout-runtime/python\${PYTHONPATH:+:\$PYTHONPATH}" | |
| else | |
| export PYTHONPATH="/sgl-workspace/sglang/python\${PYTHONPATH:+:\$PYTHONPATH}" | |
| fi |
|
Dispatched the full MI355X disaggregation workflow on |
Co-authored-by: YC Yen-Ching Tseng <yctseng@amd.com>
ce746e2 to
0aac976
Compare
…de mechanism (sgl-project#30386) Co-authored-by: bingxche <bingxche@amd.com>
Summary
sglang/sglang-routerpackages.sglangfor prefill/decode/bench.sglang-routerin the bench container before launching the router.SGLANG_USE_CHECKOUT_RUNTIME=0opt-out to use image-baked packages..git/config.max_total_tokens; cap DSV4 Flash MI355X 1k/1k configs at8551168.Tests
CI States
Latest PR Test (Base): ✅ Run #28873717185
Latest PR Test (Extra): ❌ Run #28873716286