-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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.
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
[Unified Checkpoint] Fix load best checkpoint #8935
Conversation
Thanks for your contribution! |
607be60
to
2097916
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8935 +/- ##
===========================================
+ Coverage 55.05% 55.07% +0.01%
===========================================
Files 635 635
Lines 99412 99452 +40
===========================================
+ Hits 54730 54771 +41
+ Misses 44682 44681 -1 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d1d7bc1
to
6f2b814
Compare
@@ -1203,6 +1206,8 @@ def _load_best_model_from_peft_checkpoint(self): | |||
self.state.best_model_checkpoint, | |||
safe_serialization=True, | |||
) | |||
if self.args.sharding_parallel_degree > 1 or self.args.data_parallel_degree > 1: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个是不是只需要 peft 相关的参数广播啊,这样是不是所有参数广播?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
正确性不影响吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* fix load best * update
PR types
Bug fixes
PR changes
Others
Description
Fix loading best checkpoint.