Skip to content

feat: dynamic GPU count and MoE runner backend for lora training - #2

Merged
gongyisheng merged 1 commit into
gongyisheng:miles-gpt-oss-moe-lorafrom
yushengsu-thu:miles-gpt-oss-moe-lora-yusheng
Apr 20, 2026
Merged

gongyisheng merged 1 commit into
gongyisheng:miles-gpt-oss-moe-lorafrom
yushengsu-thu:miles-gpt-oss-moe-lora-yusheng

Conversation

@yushengsu-thu

Copy link
Copy Markdown
  • Auto-detect GPUS_PER_NODE from CUDA_VISIBLE_DEVICES instead of hardcoding 4
  • Add --sglang-moe-runner-backend triton flag
  • Fix serialized_tensors to use single tensor instead of list

Made-with: Cursor

- Auto-detect GPUS_PER_NODE from CUDA_VISIBLE_DEVICES instead of hardcoding 4
- Add --sglang-moe-runner-backend triton flag
- Fix serialized_tensors to use single tensor instead of list

Made-with: Cursor
Copilot AI review requested due to automatic review settings April 18, 2026 07:10

Copilot AI 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.

Pull request overview

This PR improves LoRA training launch/config behavior by making GPU allocation dynamic, adding an MoE runner backend flag for SGLang, and adjusting LoRA weight serialization when sending tensors to the colocated rollout engine.

Changes:

  • Derive GPUS_PER_NODE from CUDA_VISIBLE_DEVICES and use it in Ray startup and training submission.
  • Add --sglang-moe-runner-backend triton to the MoE LoRA launcher script.
  • Change LoRA serialized_tensors payload from a list to a single serialized tensor string in the colocated-engine load path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
miles/backends/megatron_utils/update_weight/update_weight_from_tensor.py Adjusts LoRA adapter loading RPC payload shape for serialized tensors.
examples/lora/run-gpt-oss-20B-megatron-moe-lora.sh Uses dynamic GPU count for Ray/actor config and adds SGLang MoE runner backend flag.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +342 to +343
serialized_tensors=serialized_named_tensors[0][0],
# serialized_tensors=[serialized_named_tensors[i][0] for i in range(len(serialized_named_tensors))],

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

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

Avoid leaving commented-out previous code and trailing whitespace in this call. Now that serialized_tensors is a single string, remove the old commented list-based argument (and the extra space after the comma) to keep the call site clean and reduce ambiguity about the intended payload shape.

Suggested change
serialized_tensors=serialized_named_tensors[0][0],
# serialized_tensors=[serialized_named_tensors[i][0] for i in range(len(serialized_named_tensors))],
serialized_tensors=serialized_named_tensors[0][0],

Copilot uses AI. Check for mistakes.
# will prevent ray from buffering stdout/stderr
export PYTHONBUFFERED=16
export CUDA_VISIBLE_DEVICES=${CUDA_VISIBLE_DEVICES:-0,1,2,3}
GPUS_PER_NODE=$(echo "$CUDA_VISIBLE_DEVICES" | tr ',' '\n' | wc -l)

Copilot AI Apr 18, 2026

Copy link

Choose a reason for hiding this comment

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

GPUS_PER_NODE is now derived from CUDA_VISIBLE_DEVICES, but this script still hardcodes --rollout-num-gpus-per-engine 4 in SGLANG_ARGS. If a user sets CUDA_VISIBLE_DEVICES to a non-4 list, Ray and the actor will run with $GPUS_PER_NODE GPUs while SGLang will request 4 per engine, which is inconsistent and likely to fail. Consider setting --rollout-num-gpus-per-engine from $GPUS_PER_NODE (or add an explicit check that $GPUS_PER_NODE == 4 and exit with an error).

Suggested change
GPUS_PER_NODE=$(echo "$CUDA_VISIBLE_DEVICES" | tr ',' '\n' | wc -l)
GPUS_PER_NODE=$(echo "$CUDA_VISIBLE_DEVICES" | tr ',' '\n' | wc -l)
if [ "$GPUS_PER_NODE" -ne 4 ]; then
echo "Error: This script requires exactly 4 visible GPUs, but CUDA_VISIBLE_DEVICES='$CUDA_VISIBLE_DEVICES' resolves to $GPUS_PER_NODE GPUs." >&2
echo "Please use 4 GPUs or update the rollout/SGLang GPU settings to match GPUS_PER_NODE." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.
@gongyisheng gongyisheng self-assigned this Apr 19, 2026
@gongyisheng
gongyisheng self-requested a review April 19, 2026 23:01
@gongyisheng
gongyisheng merged commit a5861fa into gongyisheng:miles-gpt-oss-moe-lora Apr 20, 2026
3 of 4 checks passed
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