Skip to content

add DeepSpeed tensor parallel initialization. #36114

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
Closed
wants to merge 5 commits into from
Closed

add DeepSpeed tensor parallel initialization. #36114

wants to merge 5 commits into from

Conversation

inkcherry
Copy link
Contributor

@inkcherry inkcherry commented Feb 10, 2025 •
edited
Loading

Initialization for DeepSpeed tensor parallel+zero optimizer, which needs to be done before the optimizer initializes the model weights. The related APIs are already supported in DeepSpeed. deepspeedai/DeepSpeed#6922 @muellerzr

cc DeepSpeed team: @tjruwase
move to #36825

@inkcherry inkcherry mentioned this pull request Feb 24, 2025
Closed
muellerzr
muellerzr approved these changes Feb 28, 2025
View reviewed changes
Copy link
Contributor

@muellerzr muellerzr left a comment

Choose a reason for hiding this comment

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

Thanks, cc @ArthurZucker

@HuggingFaceDocBuilderDev
Copy link

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

@inkcherry
Copy link
Contributor Author

thanks for the review! @muellerzr
accelerate includes modifications related to the dataloader and saving and works together with this feature.
huggingface/accelerate#3390

@inkcherry
Copy link
Contributor Author

thanks for the review. @muellerzr! The related PR has already been merged: huggingface/accelerate#3390. Is this PR ready for merge? Thanks!

@inkcherry
Copy link
Contributor Author

inkcherry commented Mar 19, 2025 •
edited
Loading

sorry to bother you again and I hope this doesn't disturb you : ) the doc deepspeedai/DeepSpeed#7151 is ready but pending with this pr, I would appreciate it if this PR can be merged. Thank you very much!
FYI @muellerzr @ArthurZucker @shethaadit @tjruwase

@inkcherry inkcherry force-pushed the ds_tp_upstream2 branch 2 times, most recently from d3c2d1c to 24f1be4 Compare March 19, 2025 09:43
abhilash1910
abhilash1910 approved these changes Mar 19, 2025
View reviewed changes
Copy link
Contributor

@abhilash1910 abhilash1910 left a comment

Choose a reason for hiding this comment

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

LGTM ! Is the CI stalling @muellerzr ?

@inkcherry
Copy link
Contributor Author

inkcherry commented Mar 19, 2025 •
edited
Loading

LGTM ! Is the CI stalling @muellerzr ?

Not quite sure, it's been hanging on Checking for the ability to merge automatically... for several hours.
image
It's strange. Not sure if it's due to the CI or some GitHub service issue (https://www.githubstatus.com/incidents/lg4s05t6ttxb). Force pushing to refresh also results in it hanging ):

move to #36825 fix this hang issue.

@inkcherry inkcherry closed this Mar 19, 2025
@inkcherry inkcherry reopened this Mar 19, 2025
@github-actions GitHub Actions
Copy link

Hi 👋, thank you for opening this pull request! The pull request is converted to draft by default. When it is ready for review, please click the Ready for review button (at the bottom of the PR page).

@github-actions github-actions bot marked this pull request as draft March 19, 2025 14:23
@inkcherry inkcherry mentioned this pull request Mar 19, 2025
Merged
@ArthurZucker ArthurZucker marked this pull request as ready for review March 20, 2025 10:50
ArthurZucker
ArthurZucker approved these changes Mar 20, 2025
View reviewed changes
Copy link
Collaborator

@ArthurZucker ArthurZucker left a comment

Choose a reason for hiding this comment

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

Happy to merge let's make sure CIs work

@muellerzr
Copy link
Contributor

@inkcherry I think we can close this one in favor of #36825 yes?

@inkcherry
Copy link
Contributor Author

@inkcherry I think we can close this one in favor of #36825 yes?

yes, still hang now, let me close this one.

@inkcherry inkcherry closed this Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@muellerzr muellerzr muellerzr approved these changes

@shethaadit shethaadit shethaadit approved these changes

@abhilash1910 abhilash1910 abhilash1910 approved these changes

@ArthurZucker ArthurZucker ArthurZucker approved these changes

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

Successfully merging this pull request may close these issues.
6 participants
@inkcherry @HuggingFaceDocBuilderDev @muellerzr @shethaadit @abhilash1910 @ArthurZucker