Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

DS -> FSDP and back again #2133

Merged
merged 27 commits into from
Jun 13, 2024
Merged

DS -> FSDP and back again #2133

merged 27 commits into from
Jun 13, 2024

Conversation

muellerzr
Copy link
Contributor

@muellerzr muellerzr commented Jun 10, 2024 •
edited
Loading

This PR introduces a new blog collaborated between HF, PyTorch, IBM, and Contextual AI aimed at de-mystifying how DeepSpeed and Fully Sharded Data Parallelism differs. This is focused on 🤗 Accelerate specifically.

cc @stas00 @raghukiran1224 @fabianlim

cc @LysandreJik @osanseviero @stevhliu

Aiming to release by Wednesday at the latest

stevhliu
stevhliu approved these changes Jun 10, 2024
View reviewed changes
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Thanks for walking me through your investigative journey! It was really interesting to follow along 💯

deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
@muellerzr @stevhliu
Apply suggestions from code review
c583fb3 
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
stevhliu
stevhliu approved these changes Jun 10, 2024
View reviewed changes
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

LGTM, just a few nits 🙂

deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
@muellerzr @stevhliu
Apply suggestions from code review
b3a87b0 
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
muellerzr and others added 5 commits June 10, 2024 16:49
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
a52b516 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
1ba612e 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
cadac83 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
d5318c6 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
09e9aa2 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
fd2c4c2 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@raghukiran1224
Copy link
Contributor

@muellerzr all your edits look good to me!

muellerzr and others added 5 commits June 11, 2024 07:35
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
8427919 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
6293e72 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
78366dd 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
db0f43f 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr @raghukiran1224
Apply suggestions from code review
b5c2f19 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
@muellerzr muellerzr requested a review from stevhliu June 11, 2024 11:52
stevhliu
stevhliu approved these changes Jun 11, 2024
View reviewed changes
Copy link
Member

@stevhliu stevhliu left a comment

Choose a reason for hiding this comment

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

Nice! Just a few minor formattings things, otherwise LGTM!

ps. just a friendly reminder to insert all the figures. i noticed there are a few places that don't say, for example, [[ZACH: INSERT FIGURE ONE HERE]]

deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
muellerzr and others added 3 commits June 11, 2024 13:09
@muellerzr @stevhliu
Apply suggestions from code review
bfe8aff 
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
@muellerzr
Align with images
422bbf8
@muellerzr
Link to comment
422dbd3
@muellerzr @raghukiran1224
Update deepspeed-to-fsdp-and-back.md
9bad383 
Co-authored-by: Raghu Ganti <raghukiran@gmail.com>
osanseviero
osanseviero reviewed Jun 11, 2024
View reviewed changes
Copy link
Contributor

@osanseviero osanseviero left a comment

Choose a reason for hiding this comment

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

Nice!

deepspeed-to-fsdp-and-back.md Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
deepspeed-to-fsdp-and-back.md Outdated Show resolved Hide resolved
@muellerzr muellerzr merged commit 1e1d3d9 into main Jun 13, 2024
1 check passed
@muellerzr muellerzr deleted the muellerzr-ds-to-fsdp branch June 13, 2024 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raghukiran1224 raghukiran1224 raghukiran1224 left review comments

@osanseviero osanseviero osanseviero left review comments

@stevhliu stevhliu stevhliu approved these changes

@LysandreJik LysandreJik Awaiting requested review from LysandreJik

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@muellerzr @raghukiran1224 @osanseviero @stevhliu