Skip to content

Conversation

@Kunlun-Zhu
Copy link
Contributor

Fix bugs on the train_ppo.sh and adding more dependency to the requirements.txt

✅ Checks

  • My pull request adheres to the code style of this project
  • My code requires changes to the documentation
  • I have updated the documentation as required
  • All the tests have passed
  • Branch name follows type/descript (e.g. feature/add-llm-agents)
  • Ready for code review

ℹ Additional Information

Kunlun-Zhu and others added 5 commits April 25, 2025 04:13
* fix(train_ppo): 使用 setsid 和 kill PGID 改进后台服务器清理

- 使用 `setsid` 在独立的进程组中启动 AgentGym 服务器。这确保了由 `conda run` 启动的所有子进程都属于同一个组。
- 修改 `trap EXIT` 清理逻辑,使用 `kill -- -PGID` 杀死整个进程组,而不是仅杀死初始的 `conda run` PID。这样可以更可靠地终止服务器及其所有潜在的子进程。
- 将存储的 ID 从 PID 改为 PGID。
- 移除了之前检查服务器进程是否存在的逻辑,因为 `setsid` 很快退出,该检查不再可靠。

此更改解决了主脚本退出时 AgentGym 服务器可能由于信号未通过 `conda run` 正确传播而无法正确终止的问题。

TODO: 检查 train_grpo.sh 并应用类似的服务端清理逻辑。

* fix(train_ppo): 改进 AgentGym 服务器管理和启动检查
- **新增基于网络端口 (`nc`) 的服务器启动检查逻辑,替代原先不可靠的 PID 检查。**
- 实现端口检查的重试机制,并在检查失败时触发清理并退出。

---------

Co-authored-by: chenzp15 <[email protected]>
@Kunlun-Zhu Kunlun-Zhu merged commit fcef64a into main Apr 27, 2025
1 of 5 checks passed
realtmxi pushed a commit that referenced this pull request May 23, 2025
* adding new reward & delete unused modules

* Fix/agentgym server cleanup (#53)

* fix(train_ppo): 使用 setsid 和 kill PGID 改进后台服务器清理

- 使用 `setsid` 在独立的进程组中启动 AgentGym 服务器。这确保了由 `conda run` 启动的所有子进程都属于同一个组。
- 修改 `trap EXIT` 清理逻辑,使用 `kill -- -PGID` 杀死整个进程组,而不是仅杀死初始的 `conda run` PID。这样可以更可靠地终止服务器及其所有潜在的子进程。
- 将存储的 ID 从 PID 改为 PGID。
- 移除了之前检查服务器进程是否存在的逻辑,因为 `setsid` 很快退出,该检查不再可靠。

此更改解决了主脚本退出时 AgentGym 服务器可能由于信号未通过 `conda run` 正确传播而无法正确终止的问题。

TODO: 检查 train_grpo.sh 并应用类似的服务端清理逻辑。

* fix(train_ppo): 改进 AgentGym 服务器管理和启动检查
- **新增基于网络端口 (`nc`) 的服务器启动检查逻辑,替代原先不可靠的 PID 检查。**
- 实现端口检查的重试机制,并在检查失败时触发清理并退出。

---------

Co-authored-by: chenzp15 <[email protected]>

* fix config bugs

* .

* .

---------

Co-authored-by: rxdaozhang <[email protected]>
Co-authored-by: chenzp15 <[email protected]>
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