Skip to content

Add option to skip init sync#127

Draft
dl541 wants to merge 6 commits intometa-pytorch:mainfrom
dl541:skip_init_sync
Draft

Add option to skip init sync#127
dl541 wants to merge 6 commits intometa-pytorch:mainfrom
dl541:skip_init_sync

Conversation

@dl541
Copy link
Copy Markdown
Contributor

@dl541 dl541 commented Mar 10, 2025

No description provided.

dl541 added 4 commits March 8, 2025 17:03
@dl541
add option to skip initial sync in Manager
15d4fa0 
Summary:
We currently always heal on step 0 to avoid synchronization issues. We want an option to support skipping this sync for users who set the PyTorch seed so all ranks are initialized with the same values.

This diff added a init_sync boolean flag that can be passed from the manager client in python to the manager service in rust. If the manager service skips the sync depending on whether the init_sync is true.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@dl541
Fixed some rust files and tests
f803f40 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@dl541
Pass init_sync parameter in manager.py
472b8fc 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@dl541
Fix init_sync logic
e3611a1 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 10, 2025
@d4l3k
Copy link
Copy Markdown
Member

d4l3k commented Mar 10, 2025

@dl541

As promised, next steps on this PR:

  • add in a unit test for this sync logic on the rust side
  • add in a manager_test.py mocked test for the init_sync flag propagation
  • add in a manager_integ_test.py w/ seed + init_sync = False. This might be a little tricky to add asserts on correct behavior due to the multiple threads so this can be a stretch goal
  • Update PR description with a summary of this PR + a Test plan: ... section

Code pointers

dl541 added 2 commits March 11, 2025 09:23
@dl541
Add tests for manager.rs
51c43db 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@dl541
Added skip init_sync tests to python client
3dbcb78 
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@d4l3k d4l3k mentioned this pull request Apr 8, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dl541 @d4l3k @facebook-github-bot