-
Notifications
You must be signed in to change notification settings - Fork 16
Use /checkpoints instead of events parsing #312
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
base: main
Are you sure you want to change the base?
Conversation
src/together/resources/finetune.py
Outdated
@@ -551,7 +594,7 @@ def list_events(self, id: str) -> FinetuneListEvents: | |||
|
|||
return FinetuneListEvents(**response.data) | |||
|
|||
def list_checkpoints(self, id: str) -> List[FinetuneCheckpoint]: | |||
def list_checkpoints_from_events(self, id: str) -> List[FinetuneCheckpoint]: |
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.
Let's delete the old function
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.
+1
src/together/resources/finetune.py
Outdated
@@ -551,7 +594,7 @@ def list_events(self, id: str) -> FinetuneListEvents: | |||
|
|||
return FinetuneListEvents(**response.data) | |||
|
|||
def list_checkpoints(self, id: str) -> List[FinetuneCheckpoint]: | |||
def list_checkpoints_from_events(self, id: str) -> List[FinetuneCheckpoint]: |
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.
+1
src/together/resources/finetune.py
Outdated
checkpoint_path = checkpoint["path"] | ||
step = checkpoint["step"] | ||
|
||
is_final = int(step) == 0 |
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.
By the way, this feels a bit brittle — what if for some reason we did save a checkpoint at step 0? I wonder if we can add a new field to the FinetuneCheckpoint
that indicates that this is a final checkpoint
src/together/resources/finetune.py
Outdated
if checkpoint_path.endswith("_adapter"): | ||
checkpoint_type = "Final Adapter" | ||
else: | ||
checkpoint_type = "Final Merged" if had_adapters else "Final" |
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.
I think it's more correct to either detect this from the training type or move the checkpoint naming to the backend side (because you'll have to make 2 requests otherwise)
Have you read the Contributing Guidelines?
Describe your changes