Add new sync style TRAINER_DRIVEN.
#494
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2. Refactor explorer and fix the issue of inconsistent behavior between NCCL and checkpoint modes in the case of `DYNAMIC_BY_EXPLORER`.
Contributor
Summary of ChangesHello @chenyushuo, 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! This pull request enhances the system's distributed training capabilities by introducing a new synchronization style, Highlights
Using Gemini Code AssistThe 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
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 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new synchronization style DYNAMIC_BY_TRAINER and performs a significant refactoring of the synchronization logic between the explorer and trainer. The changes simplify the state machine by removing the WAITING_SYNC status and centralizing state transitions within the Synchronizer. A crucial fix is included for DYNAMIC_BY_EXPLORER mode to prevent the explorer from hanging when the trainer has no new model to sync. The tests have also been substantially refactored for better readability and maintainability, which is a great improvement.
My main feedback points are:
- A potential semantic change in the
DYNAMIC_BY_EXPLORERsync style that makes it behave like a fixed-step schedule. - A suggestion to simplify the logic in a test monkey patch to improve clarity and remove a redundant call.
Overall, this is a solid refactoring with valuable improvements.
Collaborator
Author
|
/unittest-all |
2. Bug fix and refactor in checkpoint load. 3. Fix when trainer use one gpu and memory sync method.
Collaborator
Author
|
/unittest-all |
Summary
Failed Tests
Skipped
Tests
Github Test Reporter by CTRF 💚 |
Collaborator
Author
|
/unittest-all |
Summary
Skipped
Tests
Github Test Reporter by CTRF 💚 |
…iner` to `trainer_driven` in SyncStyle
chenyushuo
changed the title
DYNAMIC_BY_TRAINER.TRAINER_DRIVEN.
Collaborator
Author
|
/unittest-all |
pan-x-c
reviewed
Feb 3, 2026
Summary
Failed Tests
Skipped
Tests
Github Test Reporter by CTRF 💚 |
Collaborator
Author
|
/unittest-all |
chenyushuo
force-pushed
the
add/dyn_by_trainer
branch
from
95afdeb to
c8f4a58
Compare
Collaborator
Author
|
/unittest-all |
Summary
Failed Tests
Skipped
Tests
Github Test Reporter by CTRF 💚 |
Collaborator
Author
|
/unittest-module-cli |
Summary
Failed Tests
Tests
Github Test Reporter by CTRF 💚 |
Collaborator
Author
|
/unittest-module-cli |
Summary
Tests
Github Test Reporter by CTRF 💚 |
pan-x-c
approved these changes
Feb 5, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
TRAINER_DRIVEN.DYNAMIC_BY_EXPLORERtoEXPLORER_DRIVENwhich maintains forward compatibility.EXPLORER_DRIVEN.chat_templatein vllm openai API server.Checklist
Please check the following items before code is ready to be reviewed.