-
Notifications
You must be signed in to change notification settings - Fork 63
[QEff. Finetune]: Adding support to load checkpoints from epoch #606
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
Conversation
Signed-off-by: Tanisha <[email protected]>
Signed-off-by: Tanisha <[email protected]>
Signed-off-by: Tanisha <[email protected]>
| intermediate_epoch = int(train_config.from_peft_checkpoint.split("/")[-2].split("_")[-1]) - 1 | ||
| intermediate_step = int(train_config.from_peft_checkpoint.split("/")[-1].split("_")[-1]) | ||
| try: | ||
| intermediate_epoch = int(train_config.from_peft_checkpoint.split("/")[-2].split("_")[-1]) - 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.
Hi @tchawada , the code handles our use case. One suggestion is current exception block is broad and catches any kind of errors. Make it more specific. Please utilize Qgenie extension which helps cover all the edge cases.
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.
Okay, I do that.
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.
@tchawada, have you checked by passing complete_epoch_2 as --from_peft_checkpoint argument. Also, try to check the existing behavior as well.
If not please do it once.
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.
Yes, I tried with the path "epoch_2" and checked the overall behavior.
Signed-off-by: Tanisha <[email protected]>
quic-meetkuma
left a comment
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. Let me know once Ann's comment is addressed.
quic-akuruvil
left a comment
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.
quic-meetkuma
left a comment
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. Let us merge it. Thanks Tanisha for quick fix. Please refer this PR in JIRA and close it. :)
Signed-off-by: Tanisha <[email protected]>
Signed-off-by: Tanisha <[email protected]> Signed-off-by: Vahid Janfaza <[email protected]>
Signed-off-by: Tanisha <[email protected]> Signed-off-by: abhishek-singh591 <[email protected]>
No description provided.