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

add checkpoint #945

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

add checkpoint #945

wants to merge 14 commits into from

Conversation

zhangsmallshark
Copy link
Contributor

support checkpoint for domino

@tjruwase tjruwase removed the request for review from awan-10 December 18, 2024 11:22
hwchen2017
hwchen2017 reviewed Dec 19, 2024
View reviewed changes
Copy link

@hwchen2017 hwchen2017 left a comment

Choose a reason for hiding this comment

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

Works as expected. I have one question I'd like to confirm. Do we need to save the status of data loader to avoid reusing data samples?

@GuanhuaWang
Copy link

Looks good to me.

cc @tjruwase

@GuanhuaWang
Copy link

GuanhuaWang commented Jan 28, 2025 •
edited
Loading

Works as expected. I have one question I'd like to confirm. Do we need to save the status of data loader to avoid reusing data samples?

@hwchen2017 , this is a good question, if this is standard in pytorch or megatron, we should keep, otherwise we can skip it.

tjruwase
tjruwase reviewed Jan 29, 2025
View reviewed changes
training/DeepSpeed-Domino/megatron/initialize.py
@@ -16,7 +16,7 @@
from megatron import get_tensorboard_writer
from megatron.core import mpu, tensor_parallel
from megatron.arguments import parse_args, validate_args
from megatron.checkpointing import load_args_from_checkpoint
# from megatron.checkpointing import load_args_from_checkpoint
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete instead of comment.

@tjruwase
Copy link
Contributor

Works as expected. I have one question I'd like to confirm. Do we need to save the status of data loader to avoid reusing data samples?

This is a reason why args is saved as part of checkpoint. I recommend following the pattern in Megatron-DeepSpeed
https://github.com/microsoft/Megatron-DeepSpeed/blob/f4157bea69f3df8c6cb66f2ebcda66ba03d1288e/megatron/checkpointing.py#L602-L611

@GuanhuaWang
Copy link

@zhangsmallshark please address above comments. Thanks!

@zhangsmallshark
Copy link
Contributor Author

@GuanhuaWang I fixed it. Please check it.

@loadams
Copy link
Contributor

loadams commented Feb 10, 2025

@zhangsmallshark - could you sign off with DCO on this PR? It replaces the CLA we had before. To fix it, the steps should be here

@zhangsmallshark
Copy link
Contributor Author

@zhangsmallshark - could you sign off with DCO on this PR? It replaces the CLA we had before. To fix it, the steps should be here

I am working on it.

kfertakis and others added 12 commits February 12, 2025 09:04
@kfertakis @tjruwase
@loadams @zhangsmallshark
Εnable reward model offloading option (deepspeedai#930)
18450b0 
* enable reward model offloading option

* fixed code formatting

* more formatting fixes

* Pre-commit formatting fix

---------

Co-authored-by: Olatunji Ruwase <[email protected]>
Co-authored-by: Logan Adams <[email protected]>
Co-authored-by: Logan Adams <[email protected]>
Signed-off-by: zhangsmallshark <[email protected]>
@Xingfu-Yi @tjruwase
@loadams @zhangsmallshark
Remove the fixed eot_token mechanism for SFT (deepspeedai#927)
216e147 
Not all pretrained LLMs use `<|endoftext|>` as the `eot_token`, therefore it's inappropriate to fix it.

Co-authored-by: Olatunji Ruwase <[email protected]>
Co-authored-by: Logan Adams <[email protected]>
Signed-off-by: zhangsmallshark <[email protected]>
@zhangsmallshark @chengming-zhang
@shenzheyu @tjruwase @loadams
DeepSpeed-Domino (deepspeedai#929)
8d143c9 
* add domino

* use transformer from deepspeed

* clean args

* mega opt

* add opt & timer

* add opt

* fix loss

* folder name

* Change arguent in pretrain script

* Add readme for domino

* Update readme for domino

* Fixing usage issues

* update dataset

* megatron dependencies

* path

* Update README.md

* remove imports

* update import

* Update README.md

* Minor example script changes

* train bash

* require

* Update README.md

---------

Co-authored-by: chengming-zhang <[email protected]>
Co-authored-by: Zheyu SHEN <[email protected]>
Co-authored-by: root <[email protected]>
Co-authored-by: Olatunji Ruwase <[email protected]>
Co-authored-by: Logan Adams <[email protected]>
Signed-off-by: zhangsmallshark <[email protected]>
@shenzheyu @zhangsmallshark
Update DeepSpeed version requirement to >=0.16.0 in requirements.txt …
600db32 
…for Domino (deepspeedai#939)

Signed-off-by: zhangsmallshark <[email protected]>
@tohtana @zhangsmallshark
Example and benchmark of APIs to offload states (deepspeedai#942)
2b906c9 
* add benchmarking for offloading states

* fix api names

Signed-off-by: zhangsmallshark <[email protected]>
@zhangsmallshark
add checkpoint
937f452 
Signed-off-by: zhangsmallshark <[email protected]>
@zhangsmallshark
fix args
6b8109e 
Signed-off-by: zhangsmallshark <[email protected]>
@simonJJJ @zhangsmallshark
remove-redundant-code (deepspeedai#947)
0be49e3 
Signed-off-by: zhangsmallshark <[email protected]>
@stceum @loadams
@tjruwase @zhangsmallshark
Add DPO support for DeepSpeed-Chat (deepspeedai#828)
f7628f4 
* Add label_smoothing while calculating step2 DPO loss in DeepSpeed-Chat.

* Add training scripts for step2 DPO in DeepSpeed-Chat.

* Remove unused packages and format the code of step2 DPO in DeepSpeed-Chat.

* Update training scripts of step2 DPO in DeepSpeed-Chat.

* Follow upstream fixes.

* Update README.md for Step2 DPO finetuning.

* Add opt 350M training log demo for step 2 dpo finetuning in DeepSpeed-Chat.

* Address the formatting issue in step2 dpo finetuning in DeepSpeed-Chat.

---------

Co-authored-by: Logan Adams <[email protected]>
Co-authored-by: Olatunji Ruwase <[email protected]>
Signed-off-by: zhangsmallshark <[email protected]>
@loadams @zhangsmallshark
Update references to torchvision (deepspeedai#949)
b436bd4 
Signed-off-by: zhangsmallshark <[email protected]>
@zhangsmallshark
save args
abfedb9 
Signed-off-by: zhangsmallshark <[email protected]>
@loadams @zhangsmallshark
Cleanup CODEOWNERS (deepspeedai#953)
d6047a5 
Signed-off-by: zhangsmallshark <[email protected]>
@zhangsmallshark
Copy link
Contributor Author

I think I fixed it. I have tried:

git rebase HEAD~10 --signoff
git push --force-with-lease origin master

@loadams
Copy link
Contributor

loadams commented Feb 12, 2025

I think I fixed it. I have tried:

git rebase HEAD~10 --signoff git push --force-with-lease origin master

It looks like you'd need to merge the DeepSpeedExamples master branch back in now. If you can't get it to work, we can override it too if you need to revert your most recent push.

@zhangsmallshark
Copy link
Contributor Author

You can override it. Thanks.

@GuanhuaWang
Copy link

GuanhuaWang commented Feb 12, 2025 •
edited
Loading

@zhangsmallshark , please resolve this branch conflict as we discussed, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjruwase tjruwase tjruwase left review comments

@hwchen2017 hwchen2017 hwchen2017 approved these changes

@GuanhuaWang GuanhuaWang Awaiting requested review from GuanhuaWang

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

Successfully merging this pull request may close these issues.