Skip to content

Conversation

@liangxiaoyun
Copy link

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

PR information

Write the detail information belongs to this PR.

Experiment results

Paste your experiment result here(if needed).

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @liangxiaoyun, 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!

此拉取请求引入了一项新功能,通过提供一键式shell脚本,极大地简化了大型语言模型的多机多卡分布式训练的设置和执行过程。它不仅提供了全面的文档,还为Qwen3-32B和Qwen2.5-72B模型提供了特定的训练脚本,旨在优化训练流程并提升用户体验。

Highlights

  • 新增分布式训练一键启动脚本: 新增了用于多机多卡分布式训练的一键启动shell脚本,极大地简化了大型语言模型的训练流程。
  • 更新文档: 更新了 Echo_README.md 文件,详细说明了分布式训练的参数、快速启动示例、Megatron并行配置建议、数据格式及工具使用方法。
  • 支持Qwen模型: 引入了 Qwen2_5_72B_agent_sft.shQwen3_32B_agent_sft.sh 两个脚本,分别用于Qwen2.5-72B和Qwen3-32B模型的分布式SFT训练。
  • 健壮的训练脚本: 训练脚本包含了健壮的错误处理、SSH连接检查、远程进程清理以及详细的日志记录功能,提升了训练的稳定性和可维护性。

🧠 New Feature in Public Preview: You can now enable Memory 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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize 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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. 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.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

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

Choose a reason for hiding this comment

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

Code Review

这个 PR 旨在增加一键启动多机多卡分布式训练的 shell 脚本,这是一个非常有用的功能。

代码审查发现了一些可以改进的地方,主要集中在脚本的可维护性、可移植性和安全性方面:

  1. 代码重复: Qwen2_5_72B_agent_sft.shQwen3_32B_agent_sft.sh 两个脚本内容基本相同,只有少数参数不同。这会导致维护困难。建议将它们合并成一个通用的启动脚本,通过命令行参数或配置文件来传递模型特定的配置(如模型路径、batch size 等)。
  2. 硬编码路径: 脚本中包含了大量用户特定的硬编码路径(例如 /data_large_v2/liangxiaoyun/...)。这使得脚本无法在其他环境或被其他用户直接使用。建议将这些路径作为脚本参数或从配置文件中读取。
  3. 安全问题: 脚本和文档中都使用了 --ssh_password 参数来传递密码,这存在严重的安全风险(密码会出现在进程列表和 shell 历史中)。强烈建议改用 SSH 密钥认证,这更安全也更方便。
  4. 文档问题: Echo_README.md 中的一些 Markdown 语法有误,并且 JSON 格式示例不正确,可能会误导用户。

具体的修改建议已在代码评论中给出。修复这些问题将大大提高脚本的质量和可用性。

Comment on lines +113 to +114
MODEL_PATH="/data_large_v2/liangxiaoyun/model_output/Qwen2.5-72B-Instruct"
LOAD_PATH="/data_large_v2/liangxiaoyun/model_output/Qwen2.5-72B-Instruct-megatron"
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

脚本中硬编码了用户 liangxiaoyun 的个人路径。这使得脚本不具有可移植性,其他用户无法直接使用。应将这些路径改为通过命令行参数传入,或者设置为更通用的默认值,并允许用户覆盖。

#!/bin/bash

# ========== 环境配置 ==========
export PYTHONPATH=\$PYTHONPATH:/data_large_v2/liangxiaoyun/projects/Megatron-LM
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

generate_train_cmd 函数内部包含了大量硬编码的路径和环境配置,例如 PYTHONPATHMEGATRON_LM_PATHMODELSCOPE_CACHE 以及 megatron 可执行文件的路径。这严重影响了脚本的可移植性。这些配置应该作为参数传递给脚本,或者在一个单独的环境配置文件中定义,由脚本 source 导入。

@@ -0,0 +1,555 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

这个脚本 (Qwen3_32B_agent_sft.sh) 与 Qwen2_5_72B_agent_sft.sh 的内容几乎完全一样,只有少数默认参数值不同。这种大量的代码重复会给未来的维护带来困难(例如,修复一个 bug 需要在两个文件中同步修改)。强烈建议将这两个脚本合并为一个通用的启动脚本。可以通过增加一个 --model_config--model_name 参数来加载不同的模型配置(如模型路径、TP/CP/PP值、batch size等),从而实现代码的复用。

| 参数 | 是否必须 | 默认值 | 说明 |
|------|----------|--------|------|
| `--ips` | 是 | - | 分布式训练的节点IP地址列表,多个IP用逗号分隔 |
| `--ssh_password` | 是 | - | 分布式训练的节点的帐户密码 |
Copy link
Contributor

Choose a reason for hiding this comment

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

high

通过命令行参数 --ssh_password 传递密码存在严重的安全风险。密码可能会被记录在 shell 历史文件中,并且在脚本运行时通过 ps 等命令被其他用户看到。建议优先使用 SSH 密钥进行免密登录,这是更安全、更推荐的做法。如果必须使用密码,也应该通过交互式提示输入,而不是作为命令行参数。

echo "必需参数:"
echo " --ips <string> 节点IP列表,逗号分隔 (与 --ip_file 二选一)"
echo " --ip_file <file> 节点IP列表文件,每行一个IP (与 --ips 二选一)"
echo " --ssh_password <string> 节点账户密码"
Copy link
Contributor

Choose a reason for hiding this comment

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

high

在帮助信息中,应当对使用 --ssh_password 参数的安全性进行警告。直接在命令行中提供密码是一种不安全的做法,密码可能会被记录在 shell 历史或进程列表中。建议推广使用 SSH 密钥。

Suggested change
echo " --ssh_password <string> 节点账户密码"
echo " --ssh_password <string> 节点账户密码 (警告: 不安全, 建议使用SSH密钥)"



## 安装
参考wiki文档:【Megatron-swift 环境安装】(https://iwiki.woa.com/p/4016971017)
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Markdown 链接语法有误。括号应使用半角括号 () 而不是全角括号 (),否则链接无法正确解析。

Suggested change
参考wiki文档:【Megatron-swift 环境安装】https://iwiki.woa.com/p/4016971017
参考wiki文档:【Megatron-swift 环境安装】(https://iwiki.woa.com/p/4016971017)

Comment on lines +83 to +87
{
"id": str "数据唯一ID",
"tools": List[Dict[str, Any]] 工具列表,
"messages": List[Dict[str, str]] 对话列表,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

此处提供的 JSON 示例格式不正确,混合了 Python 类型提示和中文描述,无法被解析。这可能会误导需要准备数据的用户。建议提供一个真实、有效的 JSON 数据示例,以帮助用户理解所需的数据结构。

Suggested change
{
"id": str "数据唯一ID",
"tools": List[Dict[str, Any]] 工具列表,
"messages": List[Dict[str, str]] 对话列表,
}
{
"id": "some_unique_id",
"tools": [{"name": "example_tool"}],
"messages": [{"role": "user", "content": "Hello!"}]
}

#
# ============================================================

# set -e
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

脚本的健壮性可以进一步提高:

  1. 建议启用 set -e (或 set -euo pipefail),使得脚本在任何命令失败时立即退出,防止后续的意外行为。
  2. cleanup 函数中,将 ssh 命令的 stderr 重定向到 /dev/null (2>/dev/null) 会隐藏潜在的清理失败信息。建议将错误信息记录到日志中,以便排查问题。
  3. cleanup 函数中的 sleep 等待是不可靠的。可以考虑使用循环检查进程是否确实已被杀死,而不是固定等待几秒。

Comment on lines +148 to +149
echo " --pipeline_model_parallel_size <int> 训练轮数 (默认: ${PP})"
echo " --context_parallel_size <int> 训练轮数 (默认: ${CP})"
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

这两个参数的说明文字 "训练轮数" 是错误的,应该是从 --epochs 参数复制粘贴过来的。请修正为对应参数的正确描述,以避免用户混淆。

Suggested change
echo " --pipeline_model_parallel_size <int> 训练轮数 (默认: ${PP})"
echo " --context_parallel_size <int> 训练轮数 (默认: ${CP})"
echo " --pipeline_model_parallel_size <int> 流水线并行度 (默认: ${PP})"
echo " --context_parallel_size <int> 上下文并行度 (默认: ${CP})"

sleep 2

# 其他配置
export TF_CPP_MIN_LOG_LEVEL=3
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

export TF_CPP_MIN_LOG_LEVEL=3 这行配置在 generate_train_cmd 函数中重复了(第 406 行已有)。可以移除重复的这一行。

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.

1 participant